Closed
Bug 635318
Opened 14 years ago
Closed 13 years ago
Duplicate accesskey in Copies & Folders settings dialog
Categories
(Thunderbird :: Preferences, defect)
Tracking
(thunderbird5.0 wanted)
RESOLVED
FIXED
Thunderbird 8.0
Tracking | Status | |
---|---|---|
thunderbird5.0 | --- | wanted |
People
(Reporter: goofy.bugzilla, Assigned: goofy.bugzilla)
References
Details
Attachments
(2 files, 1 obsolete file)
35.54 KB,
image/png
|
Details | |
718 bytes,
patch
|
bwinton
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•14 years ago
|
||
Attachment #513537 -
Flags: review?
Assignee | ||
Comment 2•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Attachment #513537 -
Flags: review? → review?(nisses.mail)
Comment 3•14 years ago
|
||
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•14 years ago
|
||
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.
Updated•14 years ago
|
Assignee: nobody → GoofyFr
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•14 years ago
|
Assignee: GoofyFr → nobody
Component: General → Preferences
QA Contact: general → preferences
Comment 6•14 years ago
|
||
why not choose "R", first letter in "replies"?
Assignee | ||
Comment 7•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
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.
Attachment #513537 -
Flags: review?(nisses.mail) → review-
Updated•14 years ago
|
Flags: wanted-thunderbird+
Updated•14 years ago
|
status-thunderbird5.0:
--- → wanted
Comment 10•14 years ago
|
||
Goofy would you feel submitting a new patch ?
Assignee | ||
Comment 11•14 years ago
|
||
Attachment #513537 -
Attachment is obsolete: true
Attachment #525970 -
Flags: review?(ludovic)
Updated•14 years ago
|
Attachment #525970 -
Flags: review?(ludovic) → review?(bwinton)
Updated•14 years ago
|
Assignee: nobody → GoofyFr
Comment 12•14 years ago
|
||
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.
Attachment #525970 -
Flags: review?(bwinton) → review+
Comment 13•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
Jim, could you please fix the missing keepArchives.accesskey entity. The wrong one breaks my localization (bug 582211#10).
Comment 17•14 years ago
|
||
(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•14 years ago
|
||
Does this still need to be checked in?
Comment 19•14 years ago
|
||
(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•13 years ago
|
||
(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•13 years ago
|
||
(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 23•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 8.0
Updated•13 years ago
|
Keywords: checkin-needed
Comment 24•13 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•