1742530 - Arrow keys and tab move focus incorrectly in Downloads panel

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P2)

Description

[James Teh [:Jamie]]

STR:

1. Ensure you have at least two downloads in the Downloads panel.
2. Open the Downloads panel.
3. Press the up and down arrow keys.
   • Expected: The download list items should get focus.
   • Actual: Buttons get focus.
4. Close the Downloads panel and open it again (to get focus onto a list item).
5. Tab through the dialog.
   • Expected: Only the buttons for the selected item should be reachable with tab.
   • Actual: Tab moves through all the buttons for all items. Worse, the first tab stop is the first button for the first download, even if a subsequent item had focus.

This problem makes the Downloads panel completely unusable for screen reader users. Even though this isn't a regression (this bug has existed for years), with the increased spotlight on the Downloads panel, I think this needs to be fixed before we ship the new Downloads panel improvements.

Comment 1

[James Teh [:Jamie]]

(In reply to James Teh [:Jamie] from comment #0)

- Expected: Only the buttons for the selected item should be reachable with tab.

In case it's helpful, an example of where we get this right with a richlistbox is the Permissions list in Settings (#permissionsBox). Here's how we manage it.

Comment 2

[Neil Deakin]

It looks like the general panel key navigation (in PanelMultiView.jsm) overrides the richlistbox key handling. Adding disablekeynav="true" to the panel makes the navigation in the list work ok.

I tried the suggestion in comment 1 and that works to handle the tab key navigating only to the button for the selected item.

However, the existing key handling interferes with the behaviour in several ways:

1. the richlistbox is already focused when the panel opens, but code at https://searchfox.org/mozilla-central/rev/6a7c3a1eda4ebb8f9c13779dbbf5eff15bacf8ed/browser/components/downloads/content/downloads.js#443 handles the cursor or tab key being pressed in an unusual way, by changing the listbox selection but not changing the focus. I think this is being done to prevent the focus indicator from appearing at first?
2. there is some special handling there as well to allow press cursor down to focus the footer row.
3. there is no rendering done for the richlistbox selection at all when disablekeynav="true" is added. But when that is false, I think the panelmutiview key navigation might be used instead and the appearance only handles the mouse moving over the items. (which shouldn't really be changing the selection, is there a reason for this?)

Comment 3

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #2)

It looks like the general panel key navigation (in PanelMultiView.jsm) overrides the richlistbox key handling. Adding disablekeynav="true" to the panel makes the navigation in the list work ok.

I tried the suggestion in comment 1 and that works to handle the tab key navigating only to the button for the selected item.

However, the existing key handling interferes with the behaviour in several ways:

1. the richlistbox is already focused when the panel opens, but code at https://searchfox.org/mozilla-central/rev/6a7c3a1eda4ebb8f9c13779dbbf5eff15bacf8ed/browser/components/downloads/content/downloads.js#443 handles the cursor or tab key being pressed in an unusual way, by changing the listbox selection but not changing the focus. I think this is being done to prevent the focus indicator from appearing at first?

I think the explanation is at https://searchfox.org/mozilla-central/rev/6a7c3a1eda4ebb8f9c13779dbbf5eff15bacf8ed/browser/components/downloads/content/downloads.js#32 .

2. there is some special handling there as well to allow press cursor down to focus the footer row.
3. there is no rendering done for the richlistbox selection at all when disablekeynav="true" is added. But when that is false, I think the panelmutiview key navigation might be used instead and the appearance only handles the mouse moving over the items. (which shouldn't really be changing the selection, is there a reason for this?)

We at least used to (unsure if this is still the case) support using keyboard shortcuts like 'del' to delete items, also when hovering items with the mouse. See in particular https://bugzilla.mozilla.org/show_bug.cgi?id=851519#c13 . So the aim was (is?) that if the user hovers an item and then hits delete, we delete the hovered item, not the default-focused item.

Comment 4

[Neil Deakin]

Attachment: Bug 1742530, support key and tab navigation within the downloads panel list, r=mak

Comment 5

[Neil Deakin]

Comment on attachment 9254665 [details]
Bug 1742530, support key and tab navigation within the downloads panel list, r=mak

I think this patch gets us into a better state with keyboard navigation.

Cursor up/down navigates the list and footer, tab navigates between the list, the button for the selected item, and the footer.

Thoughts?

Comment 6

[James Teh [:Jamie]]

Comment on attachment 9254665 [details]
Bug 1742530, support key and tab navigation within the downloads panel list, r=mak

This is so, so much better. Thank you. Two things:

1. I don't think the up/down arrow keys should wrap around. That is, if you're at the top of the list, up arrow shouldn't go to the bottom and vice versa. I realise the tab key wraps, but where that is conventional, wrapping in lists is not. (The user can always press home or end to quickly jump to the top or bottom, which I verified does work.)
2. I was initially puzzled by down arrow moving to the footer, since it's not a list item and you can get there with tab. However, on reflection, I guess this makes it really obvious to a screen reader user that there are additional items to be seen elsewhere; they might not think to tab to find that button. I flag this just to say that it's unconventional, so there's always potential for confusion there, but I think the gains outweigh the potential downsides here.

f+ with the cursor wrapping removed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9fca8e4e1bd
support key and tab navigation within the downloads panel list, r=mak,desktop-theme-reviewers,dao

Comment 9

[Alexandru Michis [:malexandru]]

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

Comment 10

[Simona Badau, Desktop QA]

Wanted to verify the fix on Firefox 97 beta 4 and these are the behaviors I'm seeing on a downloads panel where 5 downloads were made:

1. Pressing the Up key after opening the downloads panel only focuses the first top download. Navigation through the rest of the downloads is not done nor resumed (as it is the Open Application Menu) - please see the screencast.

2. Pressing the Down key after opening the downloads panel focuses directly the second top download. Navigation through the rest of the downloads stops when reaching "Show all downloads", it is not resumed as it is done in the Open Application Menu for example. - see the screencast.

3. Pressing the Tab key focuses the "Open in finder" button from the first top download and the "Show all downloads" button, jumps over the rest of the downloads. - please see the screencast.

Neil, are these behaviors intended?

Comment 11

[Neil Deakin]

1. Pressing the Up key after opening the downloads panel only focuses the first top download. Navigation through the rest of the downloads is not done nor resumed (as it is the Open Application Menu) - please see the screencast.

Comment 6 implied that the navigation should not wrap around at the end.

2. Pressing the Down key after opening the downloads panel focuses directly the second top download. Navigation through the rest of the downloads stops when reaching "Show all downloads", it is not resumed as it is done in the Open Application Menu for example. - see the screencast.

That probably should be cleaned up in some manner. I think the issue here is that the first item defaults to being selected when downloads is opened but no focus indicator of this is shown. So pressing down then moves to the second item.

3. Pressing the Tab key focuses the "Open in finder" button from the first top download and the "Show all downloads" button, jumps over the rest of the downloads. - please see the screencast.

Pressing Tab should cycle between three things: 1. focus on the download list. 2. focus on the button for the selected row . 3. The footer 'show all downloads' button.

Comment 12

[Simona Badau, Desktop QA]

(In reply to Neil Deakin from comment #11)

1. Pressing the Up key after opening the downloads panel only focuses the first top download. Navigation through the rest of the downloads is not done nor resumed (as it is the Open Application Menu) - please see the screencast.

Comment 6 implied that the navigation should not wrap around at the end.

Just to make sure I've drawn the right conclusion, pressing the Up arrow key should now focus on the first top download and not start from the end (starting from the "Show all downloads" and up) as it used to on FX 96, right?

2. Pressing the Down key after opening the downloads panel focuses directly the second top download. Navigation through the rest of the downloads stops when reaching "Show all downloads", it is not resumed as it is done in the Open Application Menu for example. - see the screencast.

That probably should be cleaned up in some manner. I think the issue here is that the first item defaults to being selected when downloads is opened but no focus indicator of this is shown. So pressing down then moves to the second item.

Logged Bug 1751014 to cover this.

Comment 13

[Neil Deakin]

Just to make sure I've drawn the right conclusion, pressing the Up arrow key should now focus on the first top download and not start from the end (starting from the "Show all downloads" and up) as it used to on FX 96, right?

That is correct.

Comment 14

[Simona Badau, Desktop QA]

(In reply to Neil Deakin from comment #13)

Just to make sure I've drawn the right conclusion, pressing the Up arrow key should now focus on the first top download and not start from the end (starting from the "Show all downloads" and up) as it used to on FX 96, right?

That is correct.

Thanks, Neil for all the support!

Based on Comment 11, 12, and 13, marking this issue as Verified.