Open Bug 1228802 Opened 8 years ago Updated 10 months ago

`focus` event shouldn't be fired at the `document` object

Categories

(Core :: DOM: Events, defect, P5)

defect

Tracking

()

Tracking Status
firefox45 --- affected

People

(Reporter: mozilla, Assigned: smaug)

References

(Blocks 1 open bug, )

Details

(6 keywords)

Attachments

(1 file)

Upstream Bootstrap bug report:
https://github.com/twbs/bootstrap/issues/18365

Steps to reproduce:
1. Open http://jsfiddle.net/cvrebert/mLq3vo6f/show/ in Firefox
2. Click into the <input> on the webpage

Expected console output (which is given by Chrome, Safari, Edge, IE11)
(modulo `focusin` events because bug 687787):
focus CAPTURE Window
focus WINDOW
focus CAPTURE <input type=​"text" id=​"q" value=​"Foo">​
focus INPUT

Actual console output in Firefox:
focus CAPTURE HTMLDocument
focus DOCUMENT
focus CAPTURE Window
focus WINDOW
focus CAPTURE <input type="text" id="q" value="Foo">
focus INPUT


This indicates that Firefox is firing `focus` events at the `document` object (HTMLDocument),
which should be impossible under the HTML5 "focus update steps" algorithm (https://html.spec.whatwg.org/multipage/interaction.html#focus-update-steps ),
which says:
> If *entry* is a Document object, let *`focus` event target* be that Document object's Window object.

Additionally, this behavior doesn't match the behavior of Chrome, Safari, Edge, or IE11,
all of which don't fire `focus` events at the `document` object.

Tested in Firefox 44.0a2 (2015-11-28) on OS X 10.11.1.
Component: Untriaged → DOM: Events
Whiteboard: [parity-ie][parity-edge][parity-chrome][parity-opera][parity-safari]
Blocks: 1230801
We had to ship a workaround for this in Bootstrap. :-(
https://github.com/twbs/bootstrap/pull/18638
This bug is present since at least OS X Firefox 3.7a5pre (2010-06-06 030908) (despite a few tries, couldn't find an earlier build in mozregression that'd open without crashing), so this isn't a regression.
This is _super_ regression risky. But I guess we should try it. However it is possible that we just can't change the behavior here, it might break too many web sites.

(Of course it is trivial to filter out these focus events in scripts.)
Assignee: nobody → bugs
(In reply to Olli Pettay [:smaug] from comment #5)
> This is _super_ regression risky. But I guess we should try it. However it
> is possible that we just can't change the behavior here, it might break too
> many web sites.


We should try to assess this. :) It's an interesting bug. The risk I see is that if we are the only one to do this we break workarounds based on user agent sniffing. (to check).
Attached patch patchSplinter Review
Let's see how many tests break. At least the browser starts up with this and seems to even work :)



https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa9d1fd1ec90
Flags: needinfo?(bugs)
Looks like the Try job got stuck at ~90% complete?

I've no familiarity with Gecko internals, but FWIW: The error messages from some of the failed tests seem to suggest there's some chrome (cursor, contenteditable ?) and extension stuff which might be using these events. Perhaps the event could be made to fire in a way that's only visible to chrome/extension content? IDK.
Has Regression Range: --- → irrelevant
Has STR: --- → yes
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Whiteboard: [parity-ie][parity-edge][parity-chrome][parity-opera][parity-safari]
Priority: -- → P5

smaug, what happened here? When debugging another bug, I came across a comment saying that the check that's the problem could be removed if this bug was fixed.

Flags: needinfo?(bugs)
Severity: normal → S3

I think we should add some telemetry for this, and if people aren't using the document level focus too much, then remove

Flags: needinfo?(smaug)
You need to log in before you can comment on or make changes to this bug.