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

RESOLVED FIXED in Thunderbird 3.0b3

Status

Thunderbird
Preferences
P4
normal
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: philor, Assigned: sipaq)

Tracking

Trunk
Thunderbird 3.0b3
Bug Flags:
wanted-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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 http://developer.mozilla.org/En/XUL_Accesskey_FAQ_and_Policies 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?

Updated

9 years ago
Flags: wanted-thunderbird3? → wanted-thunderbird3+
Keywords: helpwanted
Priority: -- → P4
Whiteboard: [good first bug]
Target Milestone: --- → Thunderbird 3.0b2
(Assignee)

Comment 1

8 years ago
Created attachment 360467 [details] [diff] [review]
Patch v1

This is simple enough, so that I can take it. Maybe we can even get this in for beta2.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #360467 - Flags: review?(philringnalda)
(Reporter)

Updated

8 years ago
Attachment #360467 - Flags: review?(philringnalda) → review-
(Reporter)

Comment 2

8 years ago
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.
(Reporter)

Comment 3

8 years ago
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]
(Assignee)

Comment 4

8 years ago
Created attachment 361133 [details] [diff] [review]
Patch v2

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)
(Reporter)

Comment 5

8 years ago
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'
(Assignee)

Comment 7

8 years ago
http://hg.mozilla.org/comm-central/rev/497ca78a30f5 pushed.

Thanks for the reviews, guys!
Status: ASSIGNED → RESOLVED
Last Resolved: 8 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.