570835 - Focus should be removed from elements when they are hidden or otherwise unfocusable

Status: NEW

(Core :: DOM: Core & HTML, defect, P2)

Description

[Ojan Vafai]

Attachment: test case

If display:none is set on the focused element, then document.activeElement should no longer return that element. The element no longer receives key events, so it behaves as if it's not focused. document.activeElement should reflect that.

Comment 1

[Ojan Vafai]

Attachment: fixed test case

Comment 2

[Ojan Vafai]

Related webkit bug: https://bugs.webkit.org/show_bug.cgi?id=40338

Comment 3

[Boris Zbarsky [:bzbarsky]]

Neil, sounds like we should move focus if the focused element loses its frame.  That said, what happens if the loss of frame is temporary?

Comment 4

[Ojan Vafai]

(In reply to comment #3)
> Neil, sounds like we should move focus if the focused element loses its frame. 
> That said, what happens if the loss of frame is temporary?

This is all ill-defined grey area, but my intuition is that it's OK that toggling display would lose focus as long as it doesn't turn out to cause a compatibility problem.

I think it would also be OK if display:none elements retained focus, but if they do so, they should behave as if they have focus (e.g. receive key events).

It's not clear to me which behavior is better.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Frame changes (render object changes in Webkit terms) happen on more than just display changes, though.  In Gecko, they happen at least on changes to list-style-position, box-ordinal, column-width, column-count, table rules, table layout strategy, border-collapse, generated content, white-space, -moz-user-input.  All this on the node in question _or_ any of its ancestors.

I would be very surprised if causing changes to all of these to lose focus on the element they change on and on all descendants didn't cause compat problems.

At the same time I agree we shouldn't have focus on display:none elements....  Perhaps after restyle processing we should check whether the focused element still have a frame and if it doesn't make it lose focus then?

Comment 6

[Ojan Vafai]

(In reply to comment #5)
> Frame changes (render object changes in Webkit terms) happen on more than just
> display changes, though.  In Gecko, they happen at least on changes to
> list-style-position, box-ordinal, column-width, column-count, table rules,
> table layout strategy, border-collapse, generated content, white-space,
> -moz-user-input.  All this on the node in question _or_ any of its ancestors.

I see.

> I would be very surprised if causing changes to all of these to lose focus on
> the element they change on and on all descendants didn't cause compat problems.

Agreed.

> At the same time I agree we shouldn't have focus on display:none elements.... 
> Perhaps after restyle processing we should check whether the focused element
> still have a frame and if it doesn't make it lose focus then?

Seems reasonable.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Neil, do you think that makes sense?

Comment 9

[legolas558]

Bug 374786 was clearly posted before this one, but if it's better to follow the bug in this new tracker item then OK - I'll follow this one

Thanks

Comment 10

[Hixie (not reading bugmail)]

FWIW, the spec says "When an element that is focused stops being a focusable element, or stops being focused without another element being explicitly focused in its stead, the user agent should synchronously run the focusing steps for the body element, if there is one; if there is not, then the user agent should synchronously run the unfocusing steps for the affected element only."

Also FWIW, at least WebKit doesn't do it, allowing keystrokes to go to an element after its been hidden.

Comment 14

[Dão Gottwald [:dao]]

Given that we've unintentionally been exposed to this bug repeatedly over the years in Firefox UI code, it seems likely to me that fixing this would also help a significant number of web sites out there, probably more than it would break obscure ones that somehow depend on the current behavior. I think we should give this a shot.

Comment 15

[Andrew Overholt [:overholt]]

I'll see if we can find someone to work on the "if an already-focused element becomes display:none, make it lose focus" piece of this. I'd prefer we didn't spend too much time on it so we could focus on potential content breakage before deciding whether or not to ship it.

Comment 16

[mrmr1993]

It looks like display:none elements with autofocus="autofocus" are getting focus in Nightly (2017-08-24). GitHub issues pages (e.g. [1]) have an <input id="user_login_issue" type="text" autofocus="autofocus"> which suffers from this. (Reload the page after the initial load to repro.)

Does that fall under this bug? Presumably this is a regression in the autofocus code, but it's not ideal to handle display:none  there.

[1]: https://github.com/google/google-api-go-client/issues