2034851 - Sequential focus navigation starting point should be separate from selection

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Leo Tenenbaum [:ltenenbaum]]

Attachment: testcase.html

Currently the sequential focus navigation starting point is determined by either the focused element or the selection. But it would be best to keep track of it separately, like Chrome does. Otherwise we can run into issues, such as sequential navigation resetting when the focused element becomes non-focusable (see attached test case).

Comment 2

[Leo Tenenbaum [:ltenenbaum]]

The attached test case now works as expected because in bug 2032191 we decided to move the selection when the focused element becomes non-focusable. But other browsers don't move the selection in those cases, so this would be a better way of fixing that.

Comment 3

[Leo Tenenbaum [:ltenenbaum]]

Attachment: Bug 2034851 - Separate sequential focus navigation starting point from selection. r=#dom-core

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Is it? In general it seems nice that e.g. clicking or finding or selecting text also moves the sequential focus navigation starting point...

Comment 5

[Leo Tenenbaum [:ltenenbaum]]

Attachment: selectionchange.html

I mean, we could also just permanently use the workaround in bug 2032191. The only issue is that we are changing the selection when the focused element becomes non-focusable or is removed, which is different from other browsers (see attached). Maybe that's not such a big issue, though.

Comment 6

[Leo Tenenbaum [:ltenenbaum]]

(And to be clear, clicking or selecting text still does set the sequential focus navigation starting point with this patch.)

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Yeah I guess changing selection as a result of focus moving is not amazing / it's rather unexpected... That said, why is the sequential focus navigation a whole nsRange? Can't it be a plain RangeBoundary? That'd simplify a bit the setup at least. When would the range not be collapsed?

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Oh you're selecting the whole node... That seems unfortunate, that triggers all sorts of funny slow paths doesn't it? Why can't it be a point, just in case it's been removed? Can't we handle that in ContentWillBeRemoved, adjusting as necessary? I guess it's not necessarily fast either...

Comment 9

[Leo Tenenbaum [:ltenenbaum]]

Yes, the problem is that we want to do slightly different things depending on whether or not the node is removed. I was just using a non-collapsed range as an easy way of detecting that, but maybe using ContentWillBeRemoved or something would be better…

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9e978fd6dad5
https://hg.mozilla.org/integration/autoland/rev/da409e4fc877
Separate sequential focus navigation starting point from selection. r=dom-core,smaug

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/59902 for changes under testing/web-platform/tests

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/da409e4fc877

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot