1681322 - Keyboard navigation in meatball menu is inconsistent

Status: VERIFIED FIXED

(Firefox :: about:logins, defect, P3)

Description

[Tim Giles [:tgiles]]

Attachment: image.png

When using the keyboard to navigate this particular menu, both the right and left arrow keys cause the menu to focus the previous item in the list.

STR:

1. Navigate to about:logins
2. Use keyboard to navigate to menu shown in image.png
3. Use right or left arrow key to navigate
4. Notice that the previous item is selected

I'm not sure what the left/right arrow keys should do in this case. In my knowledge, the left/right arrow keys should effectively do nothing when navigating a menu like this, but I don't know if there are any Firefox quirks I'm not aware of

Comment 1

[Tim Giles [:tgiles]]

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
2. Download and build the Firefox source code
   • If you have any problems, please ask on Element/Matrix in the introduction channel. They're there to help you get started.
   • You can also read the Developer Guide, which has answers to most development questions:
3. Start working on this bug.
   • You will need to modify the _focusSuccessor function in browser/components/aboutlogins/conent/components/menu-button.js
   • Currently the function only checks if the down arrow is pressed, which causes the left/up/right arrow keys to behave as the same key
     • In this menu situation, only the up and down arrow keys should perform navigation
   • You will need to pass in the event.key into the _focusSuccessor function and determine which key it is:
     • If it is the ArrowUp key, then set next to true
     • If it is the ArrowDown key, then set next to false
     • If it is any other key, return early from the function
   • If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the #lockwise-desktop channel on Element/Matrix most hours of most days.
4. Build your change with mach build and test your change with mach test browser/components/aboutlogins/tests/. Also check your changes for adherence to our style guidelines by using mach lint
5. Submit the patch (including an automated test, if applicable) for review. Mark me as a reviewer so I'll get an email to come look at your code.
   • How to Submit a Patch
6. After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!

Comment 2

[gero]

Hi,

I gladly give a patch, what you ask, but I recommend another solution, we can exclude ArrowLeft and Right at the entrypoint:
101 } else if (event.key.startsWith("Arrow")) {
102 event.preventDefault();
103 this._focusSuccessor(event.key == "ArrowDown"); <-- change to event.key == "ArrowDown" || event.key == "ArrowUp", we can save the early exit implementation and event.key pass

If it's done, i could also refactor the _focusSuccessor function a bit.

Comment 3

[gero]

My suggestion is bad. We need to change signature or exit before call _focusSuccessor it key is not up or down.

Comment 4

[Tim Giles [:tgiles]]

Hi gero, it sounds like you have a good understanding of what the patch needs to do. I'm going to go ahead and assign you the bug.

Feel free to reach out if you have questions!

Comment 5

[gero]

Attachment: Bug 1681322 - exclude left and right arrow from navigation in meatball menu. r=tgiles

The tests are not a 100% solution - hard to assert nothing happened, but they catch the problem trivially.

Comment 6

[gero]

Attachment: Bug 1681322 - Refactor menu keyboard control. r=tgiles

Depends on D99431

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28b7a2b995c3
exclude left and right arrow from navigation in meatball menu. r=tgiles

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/28b7a2b995c3

Comment 9

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this issue on the latest Nightly 86.0a1 build (Build ID: 20201218095607) on Windows 10 x64, macOS 10.15.6 and Linux Mint 20.

• Nothing happens if trying to navigate through the meatball menu using the left and right arrow keys.
• I have also verified that the navigation through the menu correctly works using the tab, up and down arrow keys.
• Also verified that the screen reader correctly reads the elements from the menu.