[webkit-reviews] review denied: [Bug 62147] Use styling test from ietestcenter fails : [Attachment 96251] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 7 10:14:40 PDT 2011


Nikolas Zimmermann <zimmermann at kde.org> has denied Rob Buis
<rwlbuis at gmail.com>'s request for review:
Bug 62147: Use styling test from ietestcenter fails
https://bugs.webkit.org/show_bug.cgi?id=62147

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96251&action=review

r- because of some style and build issues and some remaining questions:

> Source/WebCore/css/CSSStyleSelector.cpp:1295
> +	   // TODO: shadow root?

What does that mean?
How does Dimitris solution handle this?

> Source/WebCore/css/CSSStyleSelector.cpp:6874
> +#if ENABLE(SVG)
> +void CSSStyleSelector::setUseElementContext(SVGUseElement *useElement)
> +{
> +    m_useElementContext = useElement;
> +}
> +#endif

Just inline it in the header.

> Source/WebCore/css/CSSStyleSelector.h:154
> +	   void setUseElementContext(SVGUseElement *);

Extra whitespace.

> Source/WebCore/rendering/svg/RenderSVGShadowTreeRootContainer.cpp:47
> +	   document()->styleSelector()->setUseElementContext(useElement);

How about introducing a SVGUseElementContextGuard, similar to your
SVGDisplayPropertyGuard in css/ that you landed a while ago, seems safer?


More information about the webkit-reviews mailing list