Duplicate access keys in the context menu of a selected link

VERIFIED FIXED in Firefox 3 beta3

Status

()

Firefox
Menus
--
minor
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

(Blocks: 1 bug, {access, polish})

Trunk
Firefox 3 beta3
access, polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

10 years ago
Created attachment 295556 [details]
Screenshot of the duplicate access keys

Steps to reproduce:

1. Make a selection containing a link.
2. Right-click on it.

The "Copy" and "Copy Link Location" items both have the same access key (C).

See the screenshot for an example of this problem.

I'll be posting a patch shortly.
(Assignee)

Updated

10 years ago
Target Milestone: --- → Firefox 3 M11
(Assignee)

Comment 1

10 years ago
Created attachment 295558 [details] [diff] [review]
Patch (v1)

The proposed solution: Use (a) as the access key.
Attachment #295558 - Flags: review?(gavin.sharp)
Comment on attachment 295558 [details] [diff] [review]
Patch (v1)

How do you find these? Perhaps we should devise a way to test this to ensure that we don't repeat this mistake in the future :)
Attachment #295558 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 3

10 years ago
Comment on attachment 295558 [details] [diff] [review]
Patch (v1)

Trivial fix, seeking approval.
Attachment #295558 - Flags: approval1.9?
(Assignee)

Comment 4

10 years ago
(In reply to comment #2)
> (From update of attachment 295558 [details] [diff] [review])
> How do you find these? Perhaps we should devise a way to test this to ensure
> that we don't repeat this mistake in the future :)

I'm personally extremely cautious of such mistakes (or leaving menu/dialog items without access keys); I found this particular one when I was working on bug 242852.

I always envisioned a tool which can maybe be implemented as a Firefox extension, and checks the menu items and dialog items for the assigned access keys at runtime and logs the items with missing (or duplicate, or inappropriate) access keys, so that they could be fixed later.  However, I've never been sure I'm proficient enough to give that a shot...

Updated

10 years ago
Attachment #295558 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Checking in browser/locales/en-US/chrome/browser/browser.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v  <--  browser.dtd
new revision: 1.84; previous revision: 1.83
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Comment 6

10 years ago
Won't this clash with the access key for "Select All" if this is present?

Though it seems this item is never shown on a link except in design mode, so that is probably not much of an issue:
http://lxr.mozilla.org/mozilla/source/browser/base/content/nsContextMenu.js#308

(Assignee)

Comment 7

10 years ago
(In reply to comment #6)
> Won't this clash with the access key for "Select All" if this is present?
> 
> Though it seems this item is never shown on a link except in design mode, so
> that is probably not much of an issue:
> http://lxr.mozilla.org/mozilla/source/browser/base/content/nsContextMenu.js#308

Good catch, thanks!  :-)

Yes, it does clash in that case.  There are also other clashes in that case as well:

 * "Cut" with "Open Link in New Tab" (T)
 * "Delete" with "Send Link..." (D)

But there is no way to solve these (at least, I don't see any way).  If someone has any ideas, I'm all for it.

But in the end, at least we've eliminated the problem in most of the cases resolving by this bug.  ;-)

Comment 8

10 years ago
(In reply to comment #7)
> But in the end, at least we've eliminated the problem in most of the cases
> resolving by this bug.  ;-)

Yes, and design mode is really a rare edge case.
(In reply to comment #6)
> Though it seems this item is never shown on a link except in design mode, so
> that is probably not much of an issue:
> http://lxr.mozilla.org/mozilla/source/browser/base/content/nsContextMenu.js#308

Good catch, I misread the condition there and missed that it would affect designmode. I agree that we're better off now than we were before, though.

Comment 10

10 years ago
Verified fixed in Gecko/2008010804 Minefield/3.0b3pre nightly build.
Status: RESOLVED → VERIFIED
Blocks: 423905

Comment 11

10 years ago
I disagree that this is a trivial fix.  You have now broken the shortcut for Copy Link Location in the many places that it is used.

I'm not sure whether anyone making these decisions is a heavy keyboard user, or if you just visually noticed the duplicate shortcuts.  As a heavy keyboard user, I would have known about the duplication and would have already learned it.  If I needed to Copy Link Location, I would right-click, 'C', Enter.  If I needed to Copy, I would right-click, 'C', 'C', Enter.

Now you have broken both of those for anyone who has been using them for years.  The "Copy" option (which isn't there very often), will take the first 'C' and the extra 'C', Enter that the user keyed may be processed by something else in the UI.

"Copy Link Location" appears in the popup menu under numerous circumstances.  Now I hit right-click, 'C' and get frustrated because Firefox won't respond.  It is a habit I've used since FF1 was in beta.  It'll take months for me to break the habit.

Even worse, I'll probably be using a mixture on FF2 and FF3 on different workstations, some of which are not my workstations, and have to pause to look at the menu in order to determine which key to hit.

Shortcut keys should be VERY CAREFULLY determined when they are implemented.  Because shortcut keys that change between versions are VERY FRUSTRATING to heavy keyboard users.
Please do any further comments in bug 423905. Thanks.
You need to log in before you can comment on or make changes to this bug.