1418097 - Reintroduce access keys for combined navigation items in context menu

Status: RESOLVED FIXED

(Firefox :: Menus, enhancement, P4)

Description

[Eitan Isaacson [:eeejay]]

In bug 1000513 access keys were removed when the navigation items were combined to an icon bar at the top. There is a lengthy discussion there about if an accesskey can be on a menu item with no text and no visual underline telling the user about the access key.

There is an accessibility justification to have the accesskeys even though we already have back/forward key shortcuts. A key shortcut requires holding a modifier key (eg. ctrl+[), this is not an option for all users.

In comment bug 1000513, comment 38, Blair mentions this not working in Linux anyway. I just did a quick test and at least now (4 years later), it works fine.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Reintroduce access keys for combined navigation items in context menu. r?jaws

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8929188 [details] [diff] [review]
Reintroduce access keys for combined navigation items in context menu. r?jaws

Review of attachment 8929188 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +187,5 @@
>  <!ENTITY hideRecentlyBookmarked.label     "Hide Recently Bookmarked">
>  <!ENTITY hideRecentlyBookmarked.accesskey "H">
>  
>  <!ENTITY backCmd.label                "Back">
> +<!ENTITY backCmd.accesskey            "B">

I don't _think_ we need to update the entity name when adding an accesskey. I'll flag flod just to make sure.

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8929188 [details] [diff] [review]
Reintroduce access keys for combined navigation items in context menu. r?jaws

Review of attachment 8929188 [details] [diff] [review]:
-----------------------------------------------------------------

Confirmed, no need to change anything if adding an access key to an existing label.

I'm not exactly sure how they can be discovered, and avoid conflicts in the same menu, but I guess that's a different problem.

Comment 4

[Eitan Isaacson [:eeejay]]

(In reply to Francesco Lodolo [:flod] (traveling, slow answers, back Nov 22) from comment #3)
> Comment on attachment 8929188 [details] [diff] [review]
> Reintroduce access keys for combined navigation items in context menu. r?jaws
> 
> Review of attachment 8929188 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Confirmed, no need to change anything if adding an access key to an existing
> label.
> 
> I'm not exactly sure how they can be discovered, and avoid conflicts in the
> same menu, but I guess that's a different problem.

Screen reader users will still hear the access key when they focus the button.

FWIW, I just checked on Chrome in Linux, those access keys work but they are not shown (no underline).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae8d16930334
Reintroduce access keys for combined navigation items in context menu. r=jaws

Comment 6

[Francesco Lodolo [:flod]]

(In reply to Eitan Isaacson [:eeejay] from comment #4)
> Screen reader users will still hear the access key when they focus the
> button.
> 
> FWIW, I just checked on Chrome in Linux, those access keys work but they are
> not shown (no underline).

Can someone double check that we're not breaking locales like Japanese? It would be enough to set intl.menuitems.alwaysappendaccesskeys to true in about:config

Reference:
https://hg.mozilla.org/l10n/gecko-strings/file/default/toolkit/chrome/global/intl.properties#l60

Comment 7

[Cristian Brindusan [:cbrindusan]]

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