562776 - Buttons in digest pane are using the same access key multiple times

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

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

Attachment: screenshot

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100427 Minefield/3.7a5pre

When you have the digest pane open for any type of add-on open, the same access key is used for the Enable, Disable, Remove buttons. We have to avoid that. An access key should only be used once.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Sadly, the solution to that problem is to completely remove the access keys.

Comment 2

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

As long as we wanna show the buttons for all add-ons in that list even when the add-on isn't selected, it's true.

Comment 3

[Jennifer Morrow [:Boriss] (UX)]

I'm removing uiwanted - seems to be on dev side

Comment 4

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

Jennifer, the reason why I have added the uiwanted keyword is, if we really wanna show up the buttons for all entries, even when they are not selected. In the old ui only the selected items shows buttons for enabling/disabling/removing an extension.

Comment 5

[Marco Zehe (:MarcoZ)]

Bug 563912 is closely related to this one. Having to havigate using tab through all those buttons for all extensions makes the keyboard interaction model really cumbersome.

Comment 6

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #2)
> As long as we wanna show the buttons for all add-ons in that list even when the
> add-on isn't selected, it's true.

Agreed, we should remove the access keys.  For accessibility I'm guessing we'll need to implement a way to expand any add-on just using the keyboard, for instance by cycling focus and then having an access key just on one item.  For now let's just take out access keys.

Comment 8

[Michael Kohler [:mkohler]]

Attachment: Patch

AFAIC there are no other accesskeys to delete.

Comment 9

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

When we are going to remove the accesskeys we also have to update the dtd files, if the same access keys aren't used anywhere else.

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 447022 [details] [diff] [review]
Patch

Need to remove the relevant entities in extensions.dtd too.

Also, there's code in extensions.xml that handles the accesskey attributes for the "Show more" toggle button - that will need removed too (its in the toggleDetails method).

Comment 11

[Michael Kohler [:mkohler]]

Attachment: Patch v2

Comment 12

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

Michael, why doesn't the new patch contain the lines from patch 1? Was that by accident?

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #12)
> Michael, why doesn't the new patch contain the lines from patch 1? Was that by
> accident?

Huh? Looks like it does include those changes.

Comment 14

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

(In reply to comment #13)
> (In reply to comment #12)
> > Michael, why doesn't the new patch contain the lines from patch 1? Was that by
> > accident?
> 
> Huh? Looks like it does include those changes.

What about extensions.xul? Don't we have to remove the enable/disable/remove accesskeys there too, which would also require to remove those from the dtd file?

Comment 15

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #14)
> What about extensions.xul? Don't we have to remove the enable/disable/remove
> accesskeys there too, which would also require to remove those from the dtd
> file?

No, that's the detail view - we don't have this problem in the detail view.

Comment 16

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/03776449f410

Comment 17

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

Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100530 Minefield/3.7a5pre (.NET CLR 3.5.30729) ID:20100530040319