[webkit-reviews] review denied: [Bug 61111] RenderText with empty text is not created inside ShadowContentElement : [Attachment 94595] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 24 08:13:05 PDT 2011


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 61111: RenderText with empty text is not created inside
ShadowContentElement
https://bugs.webkit.org/show_bug.cgi?id=61111

Attachment 94595: Patch
https://bugs.webkit.org/attachment.cgi?id=94595&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94595&action=review

cr-linux is angry plus comments:

> Source/WebCore/dom/NodeRenderingContext.cpp:56
> +    // Doesn't resolve() yet. You can do it later by calling
ensureResolved().

This comment is unnecessary.

> Source/WebCore/dom/NodeRenderingContext.cpp:59
> +void NodeRenderingContext::resolve()

determineLocation?

> Source/WebCore/dom/NodeRenderingContext.h:66
> +	   LocationUnresolved,

LocationUndetermined?

> Source/WebCore/dom/NodeRenderingContext.h:102
> +inline void NodeRenderingContext::ensureResolved()

usually this things have IfNeeded suffix, like resolveIfNeeded.

> Source/WebCore/dom/Text.cpp:202
> +    const_cast<NodeRenderingContext&>(context).ensureResolved();

This looks bad. Can we make the m_location mutable?


More information about the webkit-reviews mailing list