Closed Bug 769917 Opened 12 years ago Closed 12 years ago

Accesskey conflict in address book contact

Categories

(Thunderbird :: Address Book, defect)

defect
Not set
normal

Tracking

(thunderbird15 fixed)

RESOLVED FIXED
Thunderbird 16.0
Tracking Status
thunderbird15 --- fixed

People

(Reporter: iannbugzilla, Assigned: florian)

Details

Attachments

(1 file, 2 obsolete files)

Both the Chat tab and the Home Address field in the Photo tab use the same accesskey
http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd#130
http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd#72
The Home Address field should probably switch to "d"
I think the new Chat tab should have chosen a nonconflicting key but maybe there was no option.

I can take this but not before TB17, so if anybody can do it faster, it is free :)
We can't fix this for Thunderbird 15 as the fix requires a string change, but we should do it for Thunderbird 16.
Attached patch Patch (obsolete) — Splinter Review
(In reply to Ian Neal from comment #0)

> The Home Address field should probably switch to "d"

I agree. The Work Address field is also affected.


Mark, this patch changes strings of the en-US locale but isn't a change localizers should make in their respective locales. Would this be considered as breaking the string freeze or not? (ie can we land this on aurora before the next merge to have it fixed in Tb 15?)
Assignee: nobody → florian
Attachment #642521 - Flags: ui-review?(bwinton)
Attachment #642521 - Flags: review?(mbanner)
(In reply to Florian Quèze from comment #3)
> Created attachment 642521 [details] [diff] [review]
> Patch
> 
> (In reply to Ian Neal from comment #0)
> 
> > The Home Address field should probably switch to "d"
> 
> I agree. The Work Address field is also affected.
Yes, it is, sorry forgot to mention that.
> 
> 
> Mark, this patch changes strings of the en-US locale but isn't a change
> localizers should make in their respective locales. Would this be considered
> as breaking the string freeze or not? (ie can we land this on aurora before
> the next merge to have it fixed in Tb 15?)
The reason I spotted it was that I was doing the locale changes for en-GB, so that locale does not have the conflict.
You may find other locales either didn't have this conflict to start with or spotted it when adding the new accesskeys and fixed it.
Comment on attachment 642521 [details] [diff] [review]
Patch

I think, to avoid annoying existing users, we should change the key for the new feature, instead of changing it for the old feature.  So ui-r-.

(But you can automatically give a version that does that a ui-r=me, to save time.  ;)

Thanks,
Blake.
Attachment #642521 - Flags: ui-review?(bwinton) → ui-review-
Attached patch Patch v2 (obsolete) — Splinter Review
While discussing this on IRC with Blake, I noticed that the 't' letter doesn't seem used in any accesskey of that window.
Attachment #642521 - Attachment is obsolete: true
Attachment #642521 - Flags: review?(mbanner)
Attachment #642584 - Flags: review?(mbanner)
(In reply to Blake Winton (:bwinton - Thunderbird UX) [On vacation until July 6th!] from comment #5)
> Comment on attachment 642521 [details] [diff] [review]
> Patch
> 
> I think, to avoid annoying existing users, we should change the key for the
> new feature, instead of changing it for the old feature.  So ui-r-.
> 
> (But you can automatically give a version that does that a ui-r=me, to save
> time.  ;)
> 
> Thanks,
> Blake.

It should be noted that the only character left to use for "Chat" is "t" and it is not recommended to use lower case characters like "g,i,j,l,p,q,t" as they are not very visible.
(In reply to Florian Quèze from comment #6)

> While discussing this on IRC with Blake, I noticed that the 't' letter
> doesn't seem used in any accesskey of that window.

I missed that the same overlay is also used by the "New Contact" dialog, where the only additional accesskey is 't' http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/addressbook/abNewCardDialog.dtd#7 :-(
17:06:39 - florian: 't' doesn't work. The same overlay is also used for the "new contact" dialog where 't' is the accesskey for "Add to:" :(
17:07:13 - bwinton: Okay, fine, irc-ui-r=me. :(

So let's use attachment 642521 [details] [diff] [review] again...
Comment on attachment 642521 [details] [diff] [review]
Patch

ui-r+ per comment 9 (Blake over IRC), so requesting review from Mark again on this attachment.
Attachment #642521 - Attachment is obsolete: false
Attachment #642521 - Flags: ui-review-
Attachment #642521 - Flags: ui-review+
Attachment #642521 - Flags: review?(mbanner)
Attachment #642584 - Attachment is obsolete: true
Attachment #642584 - Flags: review?(mbanner)
Attached patch Patch v3Splinter Review
Ian requested on IRC that I make the same changes for the suite/ file with r=him for suite/.
Attachment #642521 - Attachment is obsolete: true
Attachment #642521 - Flags: review?(mbanner)
Attachment #642602 - Flags: review?(mbanner)
Comment on attachment 642602 [details] [diff] [review]
Patch v3

Requesting approval-comm-aurora too as Pike just told me on #l10n that it doesn't break the string freeze:

17:47:16 - florian: Pike: I'm wondering if the patch I have in https://bugzilla.mozilla.org/show_bug.cgi?id=769917 would be considered as breaking a string freeze or if it can land on comm-aurora, Standard8 told me you are the right person to answer this :).
17:49:46 - Pike: florian: changing accesskeys is OK
17:50:00 - Pike: florian: it'd be nice to post in .l10n as to what you're doing, and why
Attachment #642602 - Flags: approval-comm-aurora?
Attachment #642602 - Flags: review?(mbanner)
Attachment #642602 - Flags: review+
Attachment #642602 - Flags: approval-comm-aurora?
Attachment #642602 - Flags: approval-comm-aurora+
https://hg.mozilla.org/comm-central/rev/20d84b26c18a
https://hg.mozilla.org/releases/comm-aurora/rev/dae879e8e4ee
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: