[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