425176 - Change duplicated accesskeys in Composition & Addressing panels

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[Giacomo Magnini]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; it; rv:1.9b5pre) Gecko/2008032504 Minefield/3.0b5pre
Build Identifier: 

There is a duplicated accesskey (u) in the panel, and some poorly selectd ones (descendants like q and p). Patch attached.

Reproducible: Always

Comment 1

[Giacomo Magnini]

Attachment: That should be it (SM only)

Comment 2

[Misak Khachatryan]

Giacomo, did you requested review for patch ?

Comment 3

[Giacomo Magnini]

(In reply to comment #2)
> Giacomo, did you requested review for patch ?

Not before this and the other similar bugs get confirmed, or the review request will be lost.

Comment 4

[Robert Kaiser]

Comment on attachment 311781 [details] [diff] [review]
That should be it (SM only)

>Index: suite/locales/en-US/chrome/mailnews/pref/am-addressing.dtd
>===================================================================
> <!ENTITY autocompleteToMyDomain.label     "Automatically append my domain to addresses">
> <!ENTITY autocompleteToMyDomain.accesskey "u">
> <!ENTITY addressingText.label      "When looking up addresses:">
> <!-- LOCALIZATION NOTE (override.label) : do not translate "LDAP" in below line -->
> <!ENTITY useGlobal.label           "Use my global LDAP server preferences for this account">
>-<!ENTITY useGlobal.accesskey       "U">
>+<!ENTITY useGlobal.accesskey       "b">

I'd change autocompleteToMyDomain to "o" instead, so we can keep the first-letter-accesskey here, which is nicer.

>> <!ENTITY autoQuote.label                      "Automatically quote the original message when replying">
>-<!ENTITY autoQuote.accesskey                  "q">
>+<!ENTITY autoQuote.accesskey                  "m">

Why are you changing this?

> <!ENTITY then.label                           "Then,">
>-<!ENTITY then.accesskey                       "t">
>+<!ENTITY then.accesskey                       "T">

Nice (this is a slight perf improvement) ;-)

> <!ENTITY place.label                          "and place my signature">
>-<!ENTITY place.accesskey                      "p">
>+<!ENTITY place.accesskey                      "s">

And why is this change needed?

Comment 5

[Giacomo Magnini]

(In reply to comment #4)
> I'd change autocompleteToMyDomain to "o" instead, so we can keep the
> first-letter-accesskey here, which is nicer.

Agreed.

> Why are you changing this?

q (like g or p) is a descendant, which is something we should avoid as an accesskey.

> And why is this change needed?

To keep the "first-letter-accesskey" and see answer above! :-P

Comment 6

[Robert Kaiser]

Comment on attachment 311781 [details] [diff] [review]
That should be it (SM only)

OK, agreed. r=me with the mentioned change to autocompleteToMyDomain instead of useGlobal.

Comment 7

[Magnus Melin [:mkmelin]]

(In reply to comment #3)
> Not before this and the other similar bugs get confirmed, or the review request
> will be lost.

I don't think the status of the bug has any impact on review requests, best to request review from start. (If you do request review, it's even likely someone confirms it due to that.)

Comment 8

[Giacomo Magnini]

Attachment: Updated per review, with TB version as well

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 312715 [details] [diff] [review]
Updated per review, with TB version as well

r=me, thanks

Comment 10

[[e-mail]]

Comment on attachment 312715 [details] [diff] [review]
Updated per review, with TB version as well

> <!ENTITY useGlobal.label           "Use my global LDAP server preferences for this account">
>-<!ENTITY useGlobal.accesskey       "U">
>+<!ENTITY useGlobal.accesskey       "b">
Why this change?

Comment 11

[Giacomo Magnini]

Attachment: Revert useless change

Because I'm blind or dumb, pick your choice... :-P

Comment 12

[Reed Loden [:reed]]

Checking in suite/locales/en-US/chrome/mailnews/pref/am-addressing.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/pref/am-addressing.dtd,v  <--  am-addressing.dtd
new revision: 1.10; previous revision: 1.9
done
Checking in mail/locales/en-US/chrome/messenger/am-addressing.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/am-addressing.dtd,v  <--  am-addressing.dtd
new revision: 1.4; previous revision: 1.3
done

Comment 13

[Giacomo Magnini]

Verified with build id: 2008040501