635318 - Duplicate accesskey in Copies & Folders settings dialog

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Jean-Bernard Marcon]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110218 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b10pre) Gecko/20110114 Thunderbird/3.3a2

Letter C appears twice, preventing user of a keyboard to check appropriate box

Reproducible: Always

Steps to Reproduce:
1. Go to Account Settings
2.  Choose copies & Folders
3. Choices "Place replies in teh folder of the message being replied to" and "Cc these email addresses" have both the same accesskey "C"

Actual Results:  
User of keyboard only cannot choice any of these two options

Expected Results:  
another accesskey should be set (se attached patch)

Comment 1

[Jean-Bernard Marcon]

Attachment: change for accesskey "A" instead of "C"

Comment 2

[Jean-Bernard Marcon]

Attachment: duplicate C ak screenshot

Comment 3

[Andreas Nilsson (:andreasn)]

Works great on Windows and Linux so far. Need to take it on a test run on OS X as well to make sure it don't clash with any global shortcuts or anything like that before I give it an r+

Comment 4

[Anthony Ricaud (:rik)]

Looks to me like Ctrl+A is already mapped to the "Account options" select menu. At least on my Mac.

But the accesskeys doesn't show up on Mac, they are not underlined.

Comment 6

[Wayne Mery (:wsmwk)]

why not choose "R", first letter in "replies"?

Comment 7

[Jean-Bernard Marcon]

(In reply to comment #6)
> why not choose "R", first letter in "replies"?

sure it makes sense, this is the keyword. I just suggested to choose the first available letter in the string from left to right.

Comment 8

[Wayne Mery (:wsmwk)]

(In reply to comment #7)
> (In reply to comment #6)
> > why not choose "R", first letter in "replies"?
> 
> sure it makes sense, this is the keyword. 

yes, memorability is what I was looking for. It also satisfies criteria one of "Make it easy to see" of https://developer.mozilla.org/en/XUL_Accesskey_FAQ_and_Policies

Comment 9

[Andreas Nilsson (:andreasn)]

Comment on attachment 513537 [details] [diff] [review]
change for accesskey "A" instead of "C"

Agree that "R" sounds like a better shortcut, so setting r- on this.

Comment 10

[Ludovic Hirlimann [:Usul]]

Goofy would you feel submitting a new patch ?

Comment 11

[Jean-Bernard Marcon]

Attachment: accesskey R for "replies" message

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 525970 [details] [diff] [review]
accesskey R for "replies" message

I like this change, but I'm also seeing a duplicate "P" on Windows.  (For "P_lace a copy in:" and "Keep_ message archives in:")

It looks like it's a problem with mailnews/base/prefs/content/am-copiesOverlay.xul line 188-ish.  (The keepArchives checkbox is using the fccMailFolder.accesskey, which doesn't seem right.)

Would you mind adding a new access key for keepArchives, and make sure it also doesn't conflict?

(And r=me for this bit, but if you'ld like to run the other change by me, that would be cool too.)

Thanks for the patch,
Blake.

Comment 13

[Jim Porter (:squib)]

(In reply to comment #12)
> Comment on attachment 525970 [details] [diff] [review]
> accesskey R for "replies" message
> 
> I like this change, but I'm also seeing a duplicate "P" on Windows.  (For
> "P_lace a copy in:" and "Keep_ message archives in:")

That's my fault, from bug 607295. Somehow, I managed to add an accesskey for keepArchives, but then I used the wrong entity in the XUL.

Comment 14

[Ludovic Hirlimann [:Usul]]

(In reply to comment #13)
> (In reply to comment #12)
> > Comment on attachment 525970 [details] [diff] [review]
> > accesskey R for "replies" message
> > 
> > I like this change, but I'm also seeing a duplicate "P" on Windows.  (For
> > "P_lace a copy in:" and "Keep_ message archives in:")
> 
> That's my fault, from bug 607295. Somehow, I managed to add an accesskey for
> keepArchives, but then I used the wrong entity in the XUL.

Jim can you finish this ?

Comment 15

[Jim Porter (:squib)]

(In reply to comment #14)
> (In reply to comment #13)

> > That's my fault, from bug 607295. Somehow, I managed to add an accesskey for
> > keepArchives, but then I used the wrong entity in the XUL.
> 
> Jim can you finish this ?

It's just in a patch. It hasn't been committed yet. There are a couple other issues with that patch, so I'll fix all of them in one go.

Comment 16

[Pavel Franc - Mozilla.cz]

Jim, could you please fix the missing keepArchives.accesskey entity. The wrong one breaks my localization (bug 582211#10).

Comment 17

[Mark Banner (:standard8)]

(In reply to comment #16)
> Jim, could you please fix the missing keepArchives.accesskey entity. The
> wrong one breaks my localization (bug 582211#10).

That's bug 657218 which should make it into Miramar tomorrow.

Comment 18

[Jim Porter (:squib)]

Does this still need to be checked in?

Comment 19

[Ian Neal]

(In reply to comment #18)
> Does this still need to be checked in?

You could look at the equivalent bug that was done for SeaMonkey - bug 660942 where "When sending messages, automatically:" and "Place a copy in:" were changed to "When sending messages:" and "Automatically place a copy in:" respectively, allowing "u" to be the accesskey and freeing up "P" for "Place replies in the folder of the message being replied to".

If not then this patch should be using "r" rather than "R".

Comment 20

[Ludovic Hirlimann [:Usul]]

(In reply to comment #18)
> Does this still need to be checked in?

Well depedning on if you fixed what was needed to finish :-)

Comment 21

[Jim Porter (:squib)]

(In reply to comment #20)
> (In reply to comment #18)
> > Does this still need to be checked in?
> 
> Well depedning on if you fixed what was needed to finish :-)

The problems mentioned above with my code were just in a patch, and those got fixed already, so I guess that means this should be checked in too?

Comment 22

[Ludovic Hirlimann [:Usul]]

Indeed.

Comment 23

[Jonathan Protzenko [:protz]]

http://hg.mozilla.org/comm-central/rev/dd41379e2ec1

Comment 24

[Ian Neal]

(In reply to comment #23)
> http://hg.mozilla.org/comm-central/rev/dd41379e2ec1

Unfortunately did not change the accesskey from "R" to "r" to match the case in the label.