1850912 - A focus ring is displayed for a brief time on the import window when triggered from the bookmarks toolbar while the preferences page is closed

Status: NEW

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

Description

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Attachment: focus_ring.gif

Found in

• 118.0b2

Affected versions

• 119.0a1 (2023-08-30)
• 118.0b2

Tested platforms

• Affected platforms: macOS 12, Ubuntu 20, Windows 10x64,
• Unaffected platforms: none

Preconditions

• new profile

Steps to reproduce

1. Open a new tab and click on the Import bookmarks button.
2. Observe the migration window.

Expected result

• No focus ring is displayed.

Actual result

• A focus ring is briefly displayed around the migration window.

Regression range

• Last good revision: f412f38f5b6c938db981e49c1dea24f36df093f9
  First bad revision: f41311ee4ade08929b1224c893ccf12993d32ead
  Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f412f38f5b6c938db981e49c1dea24f36df093f9&tochange=f41311ee4ade08929b1224c893ccf12993d32ead

Additional notes

• Attached a screen recording.
• This only happens if the Migration window is triggered while the Preferences page is closed. Triggering it from the preferences page or while the preferences page is opened will not display the focus ring.

Comment 1

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

:sefeng, since you are the author of the regressor, bug 1811129, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Sean Feng [:sefeng]]

So in bug 1811129 we made <dialog> became focusable to have better a11y support. Apparently in this case, when the dialog was shown, it had nothing to focus within it, so the focusing algorithm focused itself. Hence the focus ring was shown. To prevent this from happening, we could change the tabIndex of the <dialog> to make it not focusable.

Alexandru, my question is, is this really an issue? I mean it seems okay to me to have the migration dialog to be focused if that's only possible thing to focus.

Comment 3

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to Sean Feng [:sefeng] from comment #2)

So in bug 1811129 we made <dialog> became focusable to have better a11y support. Apparently in this case, when the dialog was shown, it had nothing to focus within it, so the focusing algorithm focused itself. Hence the focus ring was shown. To prevent this from happening, we could change the tabIndex of the <dialog> to make it not focusable.

Alexandru, my question is, is this really an issue? I mean it seems okay to me to have the migration dialog to be focused if that's only possible thing to focus.

Hello Sean and thank you for the detailed explanation. I only considered this a consistency issue because it has different behaviors:

• Opening the migration window from the preferences page will not show the focus
• Opening the migration window from the bookmarks toolbar while on another tab and the preferences, page is opened in the background will not show the focus
• Opening the migration window from the bookmarks toolbar while the preferences page is closed will show the focus.

Feel free to close this if needed and thanks again for explaining it.

Comment 4

[Sean Feng [:sefeng]]

Ah right, it's not consistent because if the modal dialog was triggered by script (ie. Opening the migration window from the bookmarks toolbar while the preferences page is closed), the focus-visible matches. If it's triggered by keypress (ie. Opening the migration window from the preferences page), focus-visible doesn't match. I just learned this today!

This is not specific to the migration dialog, but all kinds of <dialog> element in general.

We'll discuss the best solution for this.

Comment 5

[Sean Feng [:sefeng]]

Jamie, I wonder if you have thoughts about this. A sample test case can be found at https://mozilla.seanfeng.dev/files/dialog-focus.html where the dialog gets focus ring initially, but not if you opened it by clicking the button.

Do you think we should show the focus ring or not? Always show? Always not show? Do you think we should have the different solution for this particular migration dialog and <dialog> in general?

Thanks!

Comment 6

[James Teh [:Jamie]]

(In reply to Sean Feng [:sefeng] from comment #4)

If it's triggered by keypress (ie. Opening the migration window from the preferences page), focus-visible doesn't match. I just learned this today!

If I understand this correctly, this seems kinda backwards to me. If something is opened via key press, that suggests the user is using the keyboard, so focus-visible should match, should it not? Do you get the same behaviour if you click the button with the mouse vs pressing it with the keyboard?

if the modal dialog was triggered by script (ie. Opening the migration window from the bookmarks toolbar while the preferences page is closed), the focus-visible matches.

Hmm. I guess this is because the browser can't figure out whether the user was using the keyboard or not, so it falls back to assuming that the focus should be visible? I wonder if we can somehow do better than that?

(In reply to Sean Feng [:sefeng] from comment #5)

Jamie, I wonder if you have thoughts about this. A sample test case can be found at https://mozilla.seanfeng.dev/files/dialog-focus.html where the dialog gets focus ring initially, but not if you opened it by clicking the button.

What if you press the button with the keyboard? (I can't test this myself, as I'm blind and can't see focus rings.)

Do you think we should show the focus ring or not? Always show? Always not show?

For sighted keyboard-only users, strictly speaking, I think we'd ideally always show the focus ring in all cases. However, at some point it was decided that this was too ugly, so focus-visible (and predecessors) were created as a compromise. As I understand things now:

1. We should always show the focus ring if the user accessed something via the keyboard.
2. We don't need to show it until the user uses the keyboard. That is, if we're certain they accessed something via the mouse, we probably don't need to show it.
3. If we're not sure for some reason (e.g. the user might have used the keyboard but we can't determine that for certain, maybe because of indirection due to script), we should show it so as not to break sighted keyboard-only users.

Of course, 3) is where things get tricky and where we need to use heuristics, and I suspect said heuristics are causing problems here.

NI Anna for any further thoughts, as she likely has a better grasp of focus rings than I do.

Comment 7

[Sean Feng [:sefeng]]

If I understand this correctly, this seems kinda backwards to me. If something is opened via key press, that suggests the user is using the keyboard, so focus-visible should match, should it not? Do you get the same behaviour if you click the button with the mouse vs pressing it with the keyboard?

Thanks for the input! Apparently I didn't understand this correctly. So I looked into this a bit closer and corrected my understanding. When about:preferences is not presented in any tabs, using Import bookmarks always triggers the focus ring for the modal dialog regardless the method. As you said Jamie, the heuristics can't figure how it was opened, so it used the default value.

Hmm. I guess this is because the browser can't figure out whether the user was using the keyboard or not, so it falls back to assuming that the focus should be visible? I wonder if we can somehow do better than that?

Currently the heuristics works as if we can't figure out the method, use the method of how we focus the last element. I think this worked pretty well generally, expect for this Import bookmarks case which is a bit special.

What if you press the button with the keyboard? (I can't test this myself, as I'm blind and can't see focus rings.)

Yeah, we get the focus ring.

So as a conclusion, I think our current heuristics for showing the focus ring is working well, except for cases like a navigation that triggers a new window to call showModal(), since the new window couldn't figure out how it was navigated, so it used the default value (true as the default).

So I think the correct fix is to make the newly created window knows how it was navigated.

Comment 8

[Sean Feng [:sefeng]]

Reminding myself to write a patch for this

Comment 9

[Anna Yeddi [:ayeddi]]

I'll remove my NI for now since everything has been sorted out and no new questions are posted, but feel free to NI me again if you have any concerns