[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 132233] Updated Patch

Fri Mar 16 03:10:11 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 132233: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=132233&action=review

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

The approach seems sane. Could you reorganize code a bit?

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

It looks TreeScope or ShadowRoot should have focusNode(). 
In general, it is good practice to separate finding something and modifying
something.
In this case, the function can be splited into two part:
- Checking if the focus is in the shadow tree or myself, and
- unsetting the current focus.

> LayoutTests/fast/dom/shadow/shadow-root-blur.html:1
> +<!DOCTYPE html>

This test looks great!