Closed Bug 974394 Opened 6 years ago Closed 6 years ago

Preferences: duplicated access key A used in Connection settings

Categories

(Firefox :: Preferences, defect, P5, trivial)

defect

Tracking

()

VERIFIED FIXED
Firefox 30

People

(Reporter: flod, Assigned: manishearth)

References

Details

Attachments

(1 file, 5 obsolete files)

Currently "a" is used for both "Do not prompt for authentication if password is saved" (introduced in bug 910670) and "Automatic proxy configuration URL:".

Only "i" and "v" are available, personally I would choose "v" ("i" is never a good choice for access keys).
I don't think we need an accesskey for the new "Do not prompt" pref, just added it for consistency. The pref is a one-time use thing, more or less. (The entire prefpane is like that, but this one is like that in particular)


But v would work if we want it.
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
Attached patch Removing the accesskey (obsolete) — Splinter Review
Attached patch Changing to "v" (obsolete) — Splinter Review
I'm pretty sure removing the access key is an accessibility error. Having said that, if you remove the string you need to remove the reference in the XUL file as well.
Comment on attachment 8378347 [details] [diff] [review]
Removing the accesskey

Drew, could you have a look at these and decide which one we should use? Thanks.
Attachment #8378347 - Flags: review?(adw)
Attached patch Removing the accesskey (obsolete) — Splinter Review
Adding r=
Attachment #8378347 - Attachment is obsolete: true
Attachment #8378347 - Flags: review?(adw)
Attachment #8378349 - Flags: review?(adw)
Attached patch Changing to "v" (obsolete) — Splinter Review
Attachment #8378348 - Attachment is obsolete: true
Attachment #8378350 - Flags: review?(adw)
Attachment #8378350 - Flags: review?(adw) → review+
Attachment #8378349 - Flags: review?(adw) → review-
Severity: normal → trivial
Keywords: checkin-needed
Priority: -- → P5
Comment on attachment 8378349 [details] [diff] [review]
Removing the accesskey

Even if it's r-, let's avoid possible confusions.
Attachment #8378349 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/1f826ebee2b5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Using latest Nightly in Ubuntu 13.04 Alt+V does nothing in Connection Settings panel. The access key is set to 'v' but it cannot be accessed via keyboard.
Hmm, looks like alt-v partially works if you do it from the remote DNS thing


turns out that the accesskey "v" is already used for SOCKS v5.

Should we use i, or drop it entirely?
Flags: needinfo?(adw)
Let's go with i.  Third time's the charm.
Flags: needinfo?(adw)
Attached patch replace with i (obsolete) — Splinter Review
Attachment #8378350 - Attachment is obsolete: true
Attachment #8383857 - Flags: review?(adw)
Comment on attachment 8383857 [details] [diff] [review]
replace with i

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

This patch doesn't apply cleanly because it still has "a" for the current accesskey, but the current accesskey is actually "v".  Please post a patch based on the current tree.
Attachment #8383857 - Flags: review?(adw)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch replace with iSplinter Review
Well, that was silly of me. Fixed.
Attachment #8383857 - Attachment is obsolete: true
Attachment #8383884 - Flags: review?(adw)
Comment on attachment 8383884 [details] [diff] [review]
replace with i

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

Thanks!
Attachment #8383884 - Flags: review?(adw) → review+
Keywords: checkin-needed
Tested, seems to work for me.
(In reply to Manish Goregaokar [:manishearth] from comment #12)
> turns out that the accesskey "v" is already used for SOCKS v5.

Sorry about that, not sure how I missed it while checking for free accesskeys :-\
https://hg.mozilla.org/mozilla-central/rev/c543d5d38e29
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Keywords: verifyme
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.