1983388 - Loss of focus when switching between settings categories from the keyboard

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P2)

Description

[Łukasz Golonka]

STR:

1 Open Firefox settings
2. Tab to the list of categories
3. Navigate with arrow keys to the "Home" category, notice that when categories are switched, focus remains on the list of categories
4. Press down arrow to move to the "Search" category

Actual behavior:

Focus lands on the document, rather than being still on the list of categories.

Expected behavior:

No focus loss.

Comment 1

[Alice0775 White]

Regression window:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=cada12b663001136a23f97b26aa59701d8e520d6&tochange=03b4d5426f6b2e9ddfdcb7c27640a9509cda2c1b

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:npypchenko, since you are the author of the regressor, bug 1956399, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Tim Giles [:tgiles]]

I genuinely do not understand how Bug 1956399 is the regressor here, but I'm getting the exact same regression range posted by Alice in Comment #1. Based on some quick investigation, it seems there's something going on between the gotoPref call that happens at preferences.js:234 and the gotoPref call that happens onHashChange at preferences.js:273.

I'm making this assumption based on the following:

1. I added logs and stack traces to the start of gotoPref, directly logging document.activeElement to determine when the focus seems to leave the richlistbox
2. I keyboard navigated from the "Home" button to the "Search" button in the richlistbox so that the logs would fire:

<richlistbox id="categories" data-l10n-id="category-list" data-l10n-attrs="aria-label" aria-label="Categories" allowevents="true" last-selected="category-search" keyboard-navigation="true">
preferences.js:281:20
    gotoPref chrome://browser/content/preferences/preferences.js:277
    init_all chrome://browser/content/preferences/preferences.js:234
    _fireOnSelect chrome://global/content/elements/richlistbox.js:299
    selectItem chrome://global/content/elements/richlistbox.js:440
    moveByOffset chrome://global/content/elements/richlistbox.js:757
    _moveByOffsetFromUserEvent chrome://global/content/elements/richlistbox.js:805
    RichListBox chrome://global/content/elements/richlistbox.js:32
    anonymous unknown:0
<html:body id="preferences-body" xmlns="http://www.mozilla.org/k…keeper/there.is.only.xul">
preferences.js:281:20
    gotoPref chrome://browser/content/preferences/preferences.js:277
    onHashChange chrome://browser/content/preferences/preferences.js:273
    anonymous unknown:0

Something seems to be causing the focus to move to the document before the onHashChange function is called, but I'm not sure what...

Comment 4

[James Teh [:Jamie] (away returning 14 Apr)]

This is an access s3 in that there is a workaround, but it is very annoying at best for keyboard users. In particular, less experienced keyboard or screen reader users might not know what has happened or how to deal with this situation. It is definitely a nasty papercut that a user will hit repeatedly.

Comment 5

[Tim Giles [:tgiles]]

This will most likely be fixed by Bug 1867385 as that bug is for replacing the existing navigation sidebar with the page navigation reusable component.

Comment 6

[Tim Giles [:tgiles]]

Attachment: investigation.patch

:Jamie, :smaug, I'm hoping one or both of yall have some insight as to this issue at hand. I've noticed a few things that I can't really explain, again hoping that someone will have an explanation for what is happening here.

1. For some reason, the blur and focusout events are fired when focusing the 'Search' category through keyboard navigation. I have no idea where these events are coming from and would appreciate some help in finding their source.
   • The blur and focusout events are not fired on any other category when using keyboard navigation
   • Additionally, the keydown and keyup events are fired on all the other categories. The keyup event never seems to be fired (but that's probably because the focus has moved to the body element and thus is being fired on that element instead of the richlistbox)
2. The culprit appears to be line 344 in preferences where document.location.hash is set to friendlyName
   • If I comment out this line, then focus isn't stolen for some reason. Something is happening before onHashChange is called and I can't figure out where.
   • I can't wrap my head around why changing the hash to #search is causing this focus issue and why it doesn't happen with any of the other categories.

Attached is my investigation patch for this issue if you want more context into what I've done so far. I thought the issue might have been in richlistbox, but that doesn't appear to be the case.

I'd appreciate any additional insights or steps to figure out what is going on here, thanks.

Comment 7

[Tim Giles [:tgiles]]

Here's a link to a profile that reproduces the issue. There appears to be a pretty significant gap between the keydown event that selects the "Search" category and the keyup event that fires on the "preferences-body" element with no markers being generated. Seems like the majority of the time is spent in ZwUserMsgWaitForMultipleObjectsEx which seems normal. There's only one blur event being fired so I was looking into that but I can't figure out what is making that happen from the stack.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Changing #target does change focus in general.
https://searchfox.org/firefox-main/rev/d5770dd3a8e0c22c71c8b283831ff6f410d5ff05/docshell/base/nsDocShell.cpp#9348-9349
->
https://searchfox.org/firefox-main/rev/d5770dd3a8e0c22c71c8b283831ff6f410d5ff05/docshell/base/nsDocShell.cpp#11263,11339-11340
->
https://searchfox.org/firefox-main/rev/d5770dd3a8e0c22c71c8b283831ff6f410d5ff05/layout/base/PresShell.cpp#3313,3322,3328

Is it possible that #search is focusable but other #fragments aren't or something like that?

Comment 9

[Tim Giles [:tgiles]]

Attachment: Bug 1983388 - Prevent loss of focus when navigating to search pane. r=#home-newtab-reviewers,#settings-reviewers

By changing the vbox id of the "Web Search" section, we prevent focus
from being stolen. I'm not entirely sure why this change fixes the issue
as it's likely something to do with the "7.4.6.4 Scrolling to a
fragment" part of the HTML spec. If I had to guess...it's because we end
up finding a valid focus target, due to the id of the vbox being "search".
Then we end up following the focusing steps and move the focus to the
document body.

I would bet there are no elements with ids that are the same as the hash
values we use to move between categories in about:preferences, thus
focus doesn't move because there are no valid focus targets in the
document.

See Comment #8 by :smaug for more detail.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9b843587aa46
https://hg.mozilla.org/integration/autoland/rev/5261daa46e9d
Prevent loss of focus when navigating to search pane. r=home-newtab-reviewers,akulyk,maxx,mconley

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/5261daa46e9d

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:tgiles, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox145 to wontfix.

For more information, please visit BugBot documentation.