563912 - Allow the tab key to only move to the items relevant to the selected extension

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Marco Zehe (:MarcoZ)]

STR:
1. Make sure you have two or more extensions installed.
2. In Add-Ons Manager, choose "Extensions" from the categories.
3. Tab to the list of extensions. For example if I have:

Adblock Plus
ChatZilla
GreaseMonkey

4. Choose the second, ChatZilla.
5. Press Tab.

Expected: Focus should move to "The ChatZilla Team".
Actual: It moves to whoever created the very first extension.

6. Also, when tabbing, after going through all options of Adblock Plus and ChatZilla, the options for GreaseMonkey are also being tabbed through.

With 10, 20, 30 or more extensions, this can become very cumbersome.

Comment 1

[Marco Zehe (:MarcoZ)]

This is also true for the "Search Engines", "Plugins" etc. pages.

Comment 2

[Marco Zehe (:MarcoZ)]

To be absolutely clear, this is a usability regression from the old Add-Ons manager in 3.6. In 3.6, tab would move from the list item to the buttons pertaining *only* to the selected item. So if I am at item 15, in 3.6.x, tab would not move to the buttons for item 1. In current add-ons manager, selecting item 15 in the list, then pressing Tab, will move to the buttons for item 1. So the buttons for the selected items are 30 or 45 presses of the TAB key away at least.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Talked with Marco this morning about important access related issues. In his option it's the most important one. Asking for blocking to get it on the soft blockers list. Looks like it's a really annoying regression from 3.6 for our disabled users.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

So, uh, anyone got any idea how to fix this?

Comment 5

[Dave Townsend [:mossop]]

Neil might have some ideas, I wonder if this is related to the changes that were made to differentiate between focusing with the mouse and focusing with the keyboard

Comment 6

[Neil Deakin]

When a richlistbox is focused (I'm assuming that's what is being used here), tabbing will begin from that.

nsFocusManager::DetermineElementToMoveFocus would need to be changed such that if navigating from a richlistbox, it starts from the selected item instead.

Comment 7

[Dave Townsend [:mossop]]

Ah, the reason this worked in 3.6 is that in 3.6 only the selected item had visible buttons.

Comment 8

[Neil Deakin]

In a more hackish way, you could also adjust the focusability of things in non-selected itms.

Comment 9

[Dave Townsend [:mossop]]

Attachment: patch rev 1

I likes me some hacks

Comment 10

[Dave Townsend [:mossop]]

Test fails on linux :(

Comment 11

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Passes everywhere now

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 507554 [details] [diff] [review]
patch rev 2

Delightfully hacky.

Comment 13

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/5e4632c4dd7c

Comment 14

[Dave Townsend [:mossop]]

Backed out due to test failures

Comment 15

[Marco Zehe (:MarcoZ)]

Friendly ping. This bug is still not fixed and is possibly THE most nasty keyboard nav issue we currently have in FX4, and users will view this as a productivity regression, esp now that there are so many more features in add-ons manager.

Comment 16

[Dave Townsend [:mossop]]

This bug isn't forgotten, I did some try runs yesterday to try to narrow down the cause of the test failures. The problem at the moment though is that it only fails on tinderbox not on my local machine which is making debugging the problem hard.

Comment 17

[Dave Townsend [:mossop]]

Results:

TEST-INFO | Running test 6
TEST-INFO | Focused element: {"localName":"richlistbox","id":"addon-list","className":"list"}
TEST-INFO | Expected element: {"localName":"richlistbox","id":"addon-list","className":"list"}
TEST-PASS | Focus should have moved to the list
TEST-INFO | Focused element: {"localName":"page","id":"addons-page","className":""}
TEST-INFO | Expected element: {"localName":"button","id":"","className":"details button-link"}
TEST-UNEXPECTED-FAIL | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-INFO | Focused element: {"localName":"input","id":"","className":"textbox-input"}
TEST-INFO | Expected element: {"localName":"button","id":"","className":"addon-control disable"}
TEST-UNEXPECTED-FAIL | Focus should have moved to the disable button - Got [object HTMLInputElement], expected [object XULElement]
TEST-INFO | Focused element: {"localName":"richlistbox","id":"categories","className":""}
TEST-INFO | Expected element: {"localName":"button","id":"","className":"addon-control remove"}
TEST-UNEXPECTED-FAIL | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-INFO | Focused element: {"localName":"richlistbox","id":"addon-list","className":"list"}
TEST-UNEXPECTED-FAIL | Focus should be outside the list
TEST-INFO | Focused element: {"localName":"richlistbox","id":"categories","className":""}
TEST-INFO | Expected element: {"localName":"button","id":"","className":"addon-control remove"}
TEST-UNEXPECTED-FAIL | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-INFO | Focused element: {"localName":"page","id":"addons-page","className":""}
TEST-INFO | Expected element: {"localName":"button","id":"","className":"details button-link"}
TEST-UNEXPECTED-FAIL | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-INFO | Focused element: {"localName":"richlistbox","id":"addon-list","className":"list"}
TEST-INFO | Expected element: {"localName":"richlistbox","id":"addon-list","className":"list"}
TEST-PASS | Focus should have moved to the list
TEST-INFO | Focused element: {"localName":"richlistbox","id":"categories","className":""}
TEST-PASS | Focus should be outside the list
TEST-INFO | Test 6 took 66ms

Comment 18

[Dave Townsend [:mossop]]

Relanded with a change to the test to stop paying attention to OSX full keyboard access setting, the tinderbox have it set so only textboxes and lists can get focus.

http://hg.mozilla.org/mozilla-central/rev/c99cefaa4c8e

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Marco, if you have the time it would be great when you could verify the fix and if it now works better for you. Thanks.

Comment 20

[Marco Zehe (:MarcoZ)]

This is verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110213 Firefox/4.0b12pre

However, it introduced a new bug that now traps keyboard focus at some undefined place (on the outer frame) when initially opening the add-ons manager. You have to click somewhere with the mouse to untrap it. I will file a new bug for this.

Comment 21

[Dietrich Ayala (:dietrich)]

This softblocker caused two regressions (see dependent bugs) that are both nominated for blocking.

I think it should be backed out until it's ready to land without breaking other stuff.

Comment 22

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Yes, please.  Mossop?

Comment 23

[Dave Townsend [:mossop]]

I believe it actually only caused one regression, and that one I can't reproduce for myself. Unless Marco still agrees that the other bug is worse than this I'm inclined to leave this in for now.

Comment 24

[Dave Townsend [:mossop]]

Now looks like this didn't cause any regressions so it stays in for now.