Reallocate accesskeys in prefpanes with tabs now that they're independent

RESOLVED FIXED in Thunderbird 3.0b3


11 years ago
11 years ago


(Reporter: philor, Assigned: sipaq)


Thunderbird 3.0b3
Bug Flags:
wanted-thunderbird3 +

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

Prior to the fix for bug 143065, accesskeys in the Display, Composition, Privacy, and Advanced prefpanes were shared across all tabs, so that if Advanced/General used "A" for "Automatically..." then Advanced/Update couldn't use A. That's resulted in accesskey choices that sometimes look like we'd never seen and other times like we had, and were trying our best to do the opposite. Now that we aren't stuck having to use too few letters for too many controls, we should improve things, ideally after we've gotten rid of prefs we're dropping from the UI so we don't compromise to give a good letter to something that's going away.
Flags: wanted-thunderbird3?
Flags: wanted-thunderbird3? → wanted-thunderbird3+
Keywords: helpwanted
Priority: -- → P4
Whiteboard: [good first bug]
Target Milestone: --- → Thunderbird 3.0b2
Posted patch Patch v1 (obsolete) — Splinter Review
This is simple enough, so that I can take it. Maybe we can even get this in for beta2.
Assignee: nobody → bugzilla
Attachment #360467 - Flags: review?(philringnalda)
Attachment #360467 - Flags: review?(philringnalda) → review-
Comment on attachment 360467 [details] [diff] [review]
Patch v1

Generally, omg so much nicer.

The accesskey faq probably ought to have an ordered list instead of unordered, since the rules are certain to conflict. And since a sanserif font isn't unusual on Linux (or Mac, but we don't actually show accesskeys there), the "no i or l" rule really needs to be higher on the list.

> <!-- Display and Reading Settings -->

> <!ENTITY markAsReadNoDelay.label       "Immediately on display">
>-<!ENTITY markAsReadNoDelay.accesskey   "d">
>+<!ENTITY markAsReadNoDelay.accesskey   "I">

"o" to avoid the I?

> <!-- Update -->
> <!ENTITY autoCheck.label                "Automatically check for updates to:">
> <!ENTITY enableAppUpdate.label          "&brandShortName;">
> <!ENTITY enableAppUpdate.accesskey      "h">
> <!ENTITY enableAddonsUpdate.label       "Installed Add-ons">
>-<!ENTITY enableAddonsUpdate.accesskey   "t">
>+<!ENTITY enableAddonsUpdate.accesskey   "I">
> <!ENTITY whenUpdatesFound.label         "When updates to &brandShortName; are found,">
> <!ENTITY modeAskMe.label                "Ask me what I want to do">
>-<!ENTITY modeAskMe.accesskey            "k">
>+<!ENTITY modeAskMe.accesskey            "A">

Using "A" for Installed Add-ons and "m" for Ask me not only avoids the I, it'll probably also match what Firefox will probably do when Mano probably gets around to reviewing bug 303634, which is nice when we can.

>+++ b/mail/locales/en-US/chrome/messenger/preferences/compose.dtd
> <!ENTITY inline.label                         "Inline">
> <!ENTITY inline.accesskey                     "I">
> <!ENTITY asAttachment.label                   "As Attachment">
> <!ENTITY asAttachment.accesskey               "A">

Not your fault, but those are probably the two silliest accesskeys I've ever seen. Is your fault, you're (sensibly) using A for something else in this same pane. Please make that work by removing these two accesskeys, like the person who switched from suite's radiobuttons to Tb's menulist should have done when they switched.
wladow: we both know you'll spot something that I'll feel foolish for having missed as soon as I say r+ on the next patch - want to spot it before I do, instead? :)
Keywords: helpwanted
Whiteboard: [good first bug]
Posted patch Patch v2Splinter Review
Thanks for the review, Phil. I missed the paragraph about not using "i" or "l" as an accesskey.
Attachment #360467 - Attachment is obsolete: true
Attachment #361133 - Flags: review?(philringnalda)
Comment on attachment 361133 [details] [diff] [review]
Patch v2

Looks good, thanks.
Attachment #361133 - Flags: review?(philringnalda) → review+
Comment on attachment 361133 [details] [diff] [review]
Patch v2

(In reply to comment #3)
> wladow: we both know you'll spot something that I'll feel foolish for having
> missed as soon as I say r+ on the next patch - want to spot it before I do,
> instead? :) 

Heh, I'm back home from FOSDEM now, looking at the r+ patch and guess what... :) But you don't have to feel foolish for it, really ;)  

> <!ENTITY Diskspace "Disk Space">
> <!ENTITY offlineCompact.label "Compact folders when it will save over">
>-<!ENTITY offlineCompact.accesskey "v">
>+<!ENTITY offlineCompact.accesskey "f">
> <!ENTITY kb.label "KB">

The 'f' isn't the best solution here, try to use a wider character, like 'a' or 'd' pushed.

Thanks for the reviews, guys!
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 3.0b2 → Thunderbird 3.0b3
You need to log in before you can comment on or make changes to this bug.