[webkit-reviews] review denied: [Bug 52788] Any class other than Element should not use setShadowHost() : [Attachment 89980] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 18 15:27:00 PDT 2011


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Dominic Cooney
<dominicc at chromium.org>'s request for review:
Bug 52788: Any class other than Element should not use setShadowHost()
https://bugs.webkit.org/show_bug.cgi?id=52788

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

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

Looks great, just a few nits.

> Source/WebCore/dom/EventDispatcher.cpp:136
> +	       if ((*i)->isShadowRoot() || (*i)->isSVGShadowRoot()) {

This (and hereon) should be an inline helper function.

> Source/WebCore/dom/EventDispatcher.cpp:246
> +	       ancestor = ancestor->isShadowRoot() ? ancestor->shadowHost() :
ancestor->svgShadowHost();

You are querying isShadowRoot twice here. Perhaps optimize the loop a bit?


More information about the webkit-reviews mailing list