1909885 - Pressing the SHIFT+TAB hotkey does not retain the focus inside the reader view panels

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect, P3)

Description

[Daniel Bodea [:danibodea]]

Note

• When the user navigates inside a reader view panel using TAB, the focus is retained inside the panel, but navigating backward using SHIFT+TAB beyond the first element results in moving the focus out of the panel.

Found in

• Nightly v130.0a1

Affected versions

• Nightly v130.0a1

Tested platforms

• Affected platforms: all
• Unaffected platforms: none

Steps to reproduce

1. Load an article that supports reader view.
2. Enable Reader view.
3. Using keyboard navigation open any of the panels (Text and layout, Theme, Read aloud)
4. Press SHIFT-TAB key multiple times (beyond the first element).

Expected result

• The focus is moved to the end of the panel.

Actual result

• The focus is moved out of the panel.

Regression range

• It is not a regression.

Additional information

• This part was missed from the implementation of the behavior in bug 1907280.

Comment 1

[Anna Yeddi [:ayeddi] ⛔ Out of office until November 12 ⛔]

Thank you for catching it, Daniel!

This is an access-S3 issue per accessibility triage guidelines, because it creates an inconsiderable focus order making it difficult for keyboard-only users to navigate and use the feature.

Comment 2

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

The severity field for this bug is set to S4. However, the accessibility severity is higher, .
:Gijs, could you consider increasing the severity?

For more information, please visit BugBot documentation.

Comment 3

[Irene Ni]

Labeling this as a good-first bug for a contributor with some experience, or a good-second-bug, The fix is pretty straightforward but involves a couple different files and needs unit tests.

For the narrate menu:

1. Modify the keydown event listener to remove !evt.shiftKey, since we want to listen for Shift+Tab as well.
2. In _handleFocus(), the current logic should be used if !evt.shiftKey. If evt.shiftKey is true, check if event.target is .narrate-skip-previous if narrateDropdown has the .speaking class, or .narrate-start-stop if it doesn't have the .speaking class. For either condition, .select-toggle should receive focus.

For the text and layout menu:

1. In _handleTextLayoutFocus(), attach a keydown event listener to .text-size-minus-button for Shift + Tab. When #about-reader-advanced-layout has an open attribute (meaning the advanced section is expanded), .text-layout-reset-button should receive focus. Otherwise, .accordion-header should receive focus.

For the color theme menu:

1. In _handleThemeFocus(), attach a keydown event listener to #tabs-deck-button-fxtheme for Shift + Tab. The theme button <label> that has a checked attribute (currently selected theme) should receive focus.
2. Attach another event listener to #tabs-deck-button-customtheme, where .custom-colors-reset-button should receive focus.

Refer to the patch for Bug 1907280 to see where to add unit tests.

Comment 4

[sandy.chu.40]

Hello, I'd like to be assigned to this issue

Comment 5

[:Gijs (he/him)]

(In reply to sandy.chu.40 from comment #4)

Hello, I'd like to be assigned to this issue

Thanks for your interest! We typically assign tickets for good-first-bugs when the initial version of the patch is up. :-)

Comment 6

[sandy.chu.40]

Attachment: Bug 1909885 - Add shift tab listeners for reader mode narrator, theme menu, and text menu. r=Gijs

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0444fb1a00b
Add shift tab listeners for reader mode narrator, theme menu, and text menu. r=Gijs

Comment 8

[pstanciu]

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

Comment 9

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 10

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[:Gijs (he/him)]

It's a bit late to uplift this now.

Comment 12

[Daniel Bodea [:danibodea]]

The fix works, but it uncovers a new regression: bug 1927443. This regression has a bigger impact than the issue fixed.
@sandy.chu.40: Can you look into it?

Comment 13

[Daniel Bodea [:danibodea]]

I can confirm this fix in Fx132 branch. Considering the regression mentioned in the comment above, I will be closing this report.