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)
Firefox
Menus
Tracking
()
VERIFIED
FIXED
Firefox 3 beta3
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
(Keywords: access, polish)
Attachments
(2 files)
38.21 KB,
image/png
|
Details | |
1.32 KB,
patch
|
Gavin
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
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•17 years ago
|
Target Milestone: --- → Firefox 3 M11
Assignee | ||
Comment 1•17 years ago
|
||
The proposed solution: Use (a) as the access key.
Attachment #295558 -
Flags: review?(gavin.sharp)
Comment 2•17 years ago
|
||
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•17 years ago
|
||
Comment on attachment 295558 [details] [diff] [review] Patch (v1) Trivial fix, seeking approval.
Attachment #295558 -
Flags: approval1.9?
Assignee | ||
Comment 4•17 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•17 years ago
|
Attachment #295558 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 5•17 years ago
|
||
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
Comment 6•17 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•17 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•17 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.
Comment 9•17 years ago
|
||
(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•16 years ago
|
||
Verified fixed in Gecko/2008010804 Minefield/3.0b3pre nightly build.
Status: RESOLVED → VERIFIED
Comment 11•16 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.
Comment 12•16 years ago
|
||
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.
Description
•