1907280 - The reader view panels should close when tapping ESC key

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect, P1)

Description

[Daniel Bodea [:danibodea]]

Note

• For easy keyboard navigation, the reader view panels should close when tapping the ESC key.

Found in

• Beta v129.0b2

Affected versions

• Nightly v130.0a1
• Beta v129.0b2

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 ESC key.

Expected result

• The panels should close.

Actual result

• The panels do not close.

Regression range

• Not a regression.

Additional notes

• The panel do close when tapping the UP/DOWN Arrows (the action of scrolling the page).

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hey Irene, you've worked in this area recently. How difficult do you think it'd be to wire up a handler for Esc to close the panels?

Comment 2

[Daniel Bodea [:danibodea]]

I suggest pairing this implementation with retaining the focus inside the opened panel.

For better understanding, I am providing you with these 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 the TAB key to navigate beyond the last element from the panel.
   Expected: The focus should move to the first element in the panel.
   Actual: The focus is moved out of the panel.

I believe this would ensure keyboard navigation consistency throughout the browser's panels.
I am not certain whether this behavior is desired over the current one, so I will leave this decision to you.
Please let me know if you prefer a new report for this suggestion. Thank you.

Comment 3

[Eitan Isaacson [:eeejay]]

I'm marking this as access-s2 because the ability to dismiss the popup is important because it covers content.

I agree with Daniel's focus order suggestion too.

Comment 4

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

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

For more information, please visit BugBot documentation.

Comment 5

[Irene Ni]

Attachment: Bug 1907280 - The reader view panels should close when tapping ESC key. r=#reader-mode-reviewers

Comment 6

[Irene Ni]

Apologies for the delay. I've created a patch that implements the suggested escape and focus behaviour.

Comment 7

[Donal Meehan [:dmeehan]]

Irene, next week is the final week of beta for Fx129
https://whattrainisitnow.com/release/?version=129
Are you planning on completing the patch and landing it by next week?

I'm tracking this for Fx129 since it's an S2 bug and I'm wondering if we will have a beta uplift request in time.

Comment 8

[Irene Ni]

I am on track to landing this patch before the final beta uplift date on the 26th.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03866d106200
The reader view panels should close when tapping ESC key. r=reader-mode-reviewers,cmkm

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/03866d106200

Comment 11

[Daniel Bodea [:danibodea]]

I can verify this change in Nightly v130.0a1 from 2024-07-25 in Windows 10, MacOS 11 and Ubuntu 22.
A small follow-up issue is observed and logged (bug 1909885); While the focus is retained inside the panel if the user taps TAB key, it is not retained inside the panel it the user taps SHIFT+TAB hotkey to navigate backwards.

Comment 12

[Donal Meehan [:dmeehan]]

Irene, could you add a beta uplift request on this? The final Fx129 builds tomorrow so there's not much time left.

Comment 13

[Irene Ni]

Attachment: Bug 1907280 - The reader view panels should close when tapping ESC key. r=#reader-mode-reviewers

Original Revision: https://phabricator.services.mozilla.com/D216760

Comment 14

[Donal Meehan [:dmeehan]]

Nice timing!

Comment 15

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: keyboard accessibility would be impacted
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: affects keyboard navigation within reader view menu panels only
• String changes made/needed: no
• Is Android affected?: no

Comment 16

[Donal Meehan [:dmeehan]]

Comment on attachment 9414851 [details]
Bug 1907280 - The reader view panels should close when tapping ESC key. r=#reader-mode-reviewers

Irene, the beta uplift request has conflicts.
I added a comment in the beta uplift request if you could take a look?

Comment 17

[Irene Ni]

I have updated D217714 to resolve the conflicts.

Comment 18

[Donal Meehan [:dmeehan]]

Irene, not sure if you used mozphab to update the patch. I can't land it. Pleas take a look at the comment in the Phabricator revision.

Comment 19

[Irene Ni]

Comment on attachment 9413138 [details]
Bug 1907280 - The reader view panels should close when tapping ESC key. r=#reader-mode-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: keyboard accessibility would be impacted
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: n/a
• List of other uplifts needed: Bug 1905097
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): affects keyboard navigation within reader view menu panels only
• String changes made/needed: n/a
• Is Android affected?: No

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9413138 [details]
Bug 1907280 - The reader view panels should close when tapping ESC key. r=#reader-mode-reviewers

Approved for 129.0b9

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/43b25d20d023

Comment 22

[Daniel Bodea [:danibodea]]

I can confirm this implementation in Beta v129.0b9 as well. It was verified in Windows 10, Ubuntu 22 and MacOS 11. Like in Nightly v130.0a1, it is easier to navigate by keyboard and more consistent through the browser since ESC key closes the reader view panels.
Also, the focus is retained inside the panels when tapping the TAB key beyond the last panel element, but the focus is not retained inside the panel when tapping the SHIFT+TAB hotkey beyond the first element of each of the reader view panels. This follow-up issue has been logged as bug 1909885. Thank you!