Closed Bug 410973 Opened 17 years ago Closed 17 years ago

Duplicate access keys in the context menu of a selected link

Categories

(Firefox :: Menus, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 3 beta3

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

(Keywords: access, polish)

Attachments

(2 files)

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.
Target Milestone: --- → Firefox 3 M11
Attached patch Patch (v1)Splinter Review
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+
Comment on attachment 295558 [details] [diff] [review]
Patch (v1)

Trivial fix, seeking approval.
Attachment #295558 - Flags: approval1.9?
(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...
Attachment #295558 - Flags: approval1.9? → approval1.9+
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
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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

(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.  ;-)
(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.
Verified fixed in Gecko/2008010804 Minefield/3.0b3pre nightly build.
Status: RESOLVED → VERIFIED
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.

Attachment

General

Created:
Updated:
Size: