Bug 47182

Summary: Focus remains on hidden element
Product: WebKit Reporter: doum-ti-di-li-doom <duke_the_killer>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: ap, cdumez, code.vineet, darin, kaustubh.ra, manian, m.goleb+bugzilla, rego, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://xirc.chez.com/ie9-focus-display-none.html
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=491828
https://bugzilla.mozilla.org/show_bug.cgi?id=559561
Attachments:
Description Flags
Demo none

Description doum-ti-di-li-doom 2010-10-05 08:34:56 PDT
Created attachment 69791 [details]
Demo

Focus remains on element after its parent container is styled with display:none.

Wait a second until element is hidden.
Press any key.

You will receive a message, proving now-hidden element still has focus.
Comment 1 Alexey Proskuryakov 2010-10-05 12:53:51 PDT
This is a difference with Firefox, but I don't see why this is necessarily a bad behavior.
Comment 2 doum-ti-di-li-doom 2010-10-05 19:48:21 PDT
Inconsistency is bad

An element keeps its focus after being hidden
But a hidden element cannot gain focus

In fact, Internet Explorer is like Firefox when you set style.display="none" (I was reporting the bug with that demo)

To recap:
http://xirc.chez.com/ie9-focus-1.html (focus then style.display)
Allowed by Opera / Safari

http://xirc.chez.com/ie9-focus-2.html (focus then className)
Allowed by Opera / Safari / Internet Explorer

http://xirc.chez.com/ie9-focus-3.html (style.display then focus)
Allowed by Opera

http://xirc.chez.com/ie9-focus-4.html (className then focus)
Allowed by Opera
Comment 3 Alexey Proskuryakov 2010-10-05 20:47:07 PDT
Yes, we should be different if IE and Firefox both do the same thing.
Comment 4 Kaustubh Atrawalkar 2011-09-08 00:14:12 PDT
here the issue seems to be cancelFocusAppearenceUpdate() call. Which does remove the focus appearance but does not set the focus back to 0. The current usage of this call is only in focus() & detach() calls in Element.cpp. We can add setFocus(0) in this function definition to avoid retaining of the focus after display:none. Element::blur() does the same thing, which we might able to use here.
Comment 5 Kaustubh Atrawalkar 2011-10-07 02:11:29 PDT
Is this issue to be fixed? Seems to be inconsistent on different platforms.
Comment 6 Vineet Chaudhary (vineetc) 2011-10-07 04:16:52 PDT
I think we have similar issue https://bugs.webkit.org/show_bug.cgi?id=40338 .
Comment 7 Ryosuke Niwa 2011-10-08 00:46:56 PDT
(In reply to comment #5)
> Is this issue to be fixed? Seems to be inconsistent on different platforms.

I think we should fix it. But it'll be nice to bring it up on whatwg to specify this behavior if it hasn't been done so already.
Comment 8 Ryosuke Niwa 2011-10-08 00:53:25 PDT
One tricky part of fixing this bug will be that we must fire blur event before removing the focus. Since we wouldn't know whether an element is visible or not until style recalc, it might have a weird side effects.

For example, we had decided to remove the focus when at the end of style-recalc, we might have a trouble dealing with situations like:

var element = ...

element.focus();
element.style.display = 'none';
element.title = 'hello'; // blur hasn't fired yet here
var x = element.offsetTop; // blur fires here

Because style recalc doesn't happen until offsetTop is obtained in the last line.
Comment 9 Kaustubh Atrawalkar 2011-10-08 02:18:42 PDT
Right, so if we fire the blur event and then call blur instead of cancelFocusApperance() will be right way to do it? Also, i tried with blur replacing cancelFocusApperance, but its causing few layout tests to be failed. Need to look in more detail.
Comment 10 Alexey Proskuryakov 2011-10-08 09:59:04 PDT
We should be very careful when considering changes in this area. Accepting key events on an invisible element is a fairly common technique - my understanding is that even Google Docs does something similar.
Comment 11 Ryosuke Niwa 2011-10-08 10:43:21 PDT
(In reply to comment #9)
> Right, so if we fire the blur event and then call blur instead of cancelFocusApperance() will be right way to do it? Also, i tried with blur replacing cancelFocusApperance, but its causing few layout tests to be failed. Need to look in more detail.

The real question is when to call those functions. Checking the computed value of display property of the focused node every time style changes is prohibitively inefficient and expensive. On the other hand, if we wait until style recalc happens, then there will be a noticeable delay between the time the element was hidden and blur is fired.
Comment 12 Kaustubh Atrawalkar 2012-03-07 22:12:22 PST
The whatwg mailing list thread for the issue - 
http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2012-February/034929.html
Comment 13 Ryosuke Niwa 2012-04-01 17:21:51 PDT
To paraphrase Ian's response, the spec requires the focus be dropped in this case.
Comment 14 Ryosuke Niwa 2012-04-01 17:32:22 PDT
Also see https://bugs.webkit.org/show_bug.cgi?id=40338.
Comment 15 Manuel Rego Casasnovas 2017-04-10 02:22:43 PDT
It seems only Chrome behaves like this, and the spec text seems to support that behavior:
https://bugs.chromium.org/p/chromium/issues/detail?id=491828

Issues in other bugtrackers:
* Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=559561
* Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/1308960/
Comment 16 Radar WebKit Bug Importer 2017-04-24 17:24:01 PDT
<rdar://problem/31801261>