If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

NEW
Unassigned

Status

()

Core
DOM
P2
normal
7 years ago
a month ago

People

(Reporter: Ojan Vafai, Unassigned)

Tracking

(Blocks: 3 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

860 bytes, text/html
Details
(Reporter)

Description

7 years ago
Created attachment 449969 [details]
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.
(Reporter)

Comment 1

7 years ago
Created attachment 449970 [details]
fixed test case
Attachment #449969 - Attachment is obsolete: true
(Reporter)

Comment 2

7 years ago
Related webkit bug: https://bugs.webkit.org/show_bug.cgi?id=40338
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?
(Reporter)

Comment 4

7 years ago
(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.
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?
(Reporter)

Comment 6

7 years ago
(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.
Neil, do you think that makes sense?

Updated

7 years ago
Duplicate of this bug: 374786

Updated

7 years ago
Summary: document.activeElement should not return a display:none element → Focus should be removed from elements when they are hidden

Comment 9

7 years ago
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
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
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.

Updated

5 years ago
Blocks: 752988
Blocks: 584329

Updated

4 years ago
Blocks: 935689

Updated

a year ago
Duplicate of this bug: 1181461

Updated

11 months ago
Summary: Focus should be removed from elements when they are hidden → Focus should be removed from elements when they are hidden or otherwise unfocusable

Updated

11 months ago
Duplicate of this bug: 255317

Updated

11 months ago
Duplicate of this bug: 1314760

Updated

11 months ago
Blocks: 1315950

Comment 14

11 months ago
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.
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.
Priority: -- → P3

Updated

9 months ago
See Also: → bug 1327424
Priority: P3 → P2

Comment 16

a month ago
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
You need to log in before you can comment on or make changes to this bug.