[webkit-reviews] review granted: [Bug 86707] document.activeElement should not return a non-focusable element : [Attachment 187230] Patch

[bugzilla-daemon at webkit.org]

Hajime Morrita <morrita at google.com> has granted Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 86707: document.activeElement should not return a non-focusable element
https://bugs.webkit.org/show_bug.cgi?id=86707

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

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


>>> Source/WebCore/dom/Document.cpp:3357
>>> +	 if (newFocusedNode && (newFocusedNode->isPluginElement() ||
newFocusedNode->isFocusable())) {
>> 
>> This smells wrong. The node which reaches here should be focusable IMO. We
should rather have this as an assert().
>> In the case on the test, we could just check it on
HTMLFormControlElement.cpp:focusPostAttach().
> 
> The problem is that the newFocusedNode was isFocusable at the beginning of
this function, but it became !isFocusable by blur/focusout/DOMFocusOut event
handlers.
> This issue happens with any elements with tabIndex attribute, not only form
controls.

Ah, I see the point. That makes sense.