Closed Bug 1418097 Opened 2 years ago Closed 2 years ago

Reintroduce access keys for combined navigation items in context menu

Categories

(Firefox :: Menus, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file)

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.
Attachment #8929188 - Flags: review?(jaws)
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.
Attachment #8929188 - Flags: review?(jaws)
Attachment #8929188 - Flags: review+
Attachment #8929188 - Flags: feedback?(francesco.lodolo)
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.
Attachment #8929188 - Flags: feedback?(francesco.lodolo) → feedback+
Priority: -- → P4
Keywords: checkin-needed
(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).
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae8d16930334
Reintroduce access keys for combined navigation items in context menu. r=jaws
Keywords: checkin-needed
(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
https://hg.mozilla.org/mozilla-central/rev/ae8d16930334
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.