[webkit-reviews] review denied: [Bug 81102] blur() on shadow host should work when a shadow host contains a focused element in its shadow DOM subtrees. : [Attachment 132250] Updated Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 16 04:59:16 PDT 2012


Hajime Morrita <morrita at google.com> has denied Kaustubh Atrawalkar
<kaustubh at motorola.com>'s request for review:
Bug 81102: blur() on shadow host should work when a shadow host contains a
focused element in its shadow DOM subtrees.
https://bugs.webkit.org/show_bug.cgi?id=81102

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

------- Additional Comments from Hajime Morrita <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=132250&action=review


Almost good. Could you take care of small points?

> Source/WebCore/dom/Element.cpp:1591
> +bool Element::nodeIsInShadowTree(Node* focused)

|focused| looks too specific name.
This method doesn't need to mention about focus.

> Source/WebCore/dom/Element.cpp:1594
> +    if (hasShadowRoot()) {

Let's return here.
WebKit prefers early return style.

> Source/WebCore/dom/Element.cpp:1613
> +    Node* focusedNode = doc->focusedNode();

Yeah, this looks making more sense.


More information about the webkit-reviews mailing list