Last Comment Bug 769917 - Accesskey conflict in address book contact
: Accesskey conflict in address book contact
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Address Book (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 16.0
Assigned To: Florian Quèze [:florian] [:flo]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-30 08:14 PDT by Ian Neal
Modified: 2012-07-16 09:23 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Patch (2.09 KB, patch)
2012-07-16 03:23 PDT, Florian Quèze [:florian] [:flo]
florian: ui‑review+
Details | Diff | Splinter Review
Patch v2 (1.16 KB, patch)
2012-07-16 07:44 PDT, Florian Quèze [:florian] [:flo]
no flags Details | Diff | Splinter Review
Patch v3 (4.19 KB, patch)
2012-07-16 08:45 PDT, Florian Quèze [:florian] [:flo]
standard8: review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Ian Neal 2012-06-30 08:14:50 PDT
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"
Comment 1 :aceman 2012-07-03 03:59:46 PDT
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 :)
Comment 2 Florian Quèze [:florian] [:flo] 2012-07-10 08:55:23 PDT
We can't fix this for Thunderbird 15 as the fix requires a string change, but we should do it for Thunderbird 16.
Comment 3 Florian Quèze [:florian] [:flo] 2012-07-16 03:23:36 PDT
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.


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?)
Comment 4 Ian Neal 2012-07-16 03:34:06 PDT
(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 5 Blake Winton (:bwinton) (:☕️) 2012-07-16 07:29:15 PDT
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.
Comment 6 Florian Quèze [:florian] [:flo] 2012-07-16 07:44:08 PDT
Created attachment 642584 [details] [diff] [review]
Patch v2

While discussing this on IRC with Blake, I noticed that the 't' letter doesn't seem used in any accesskey of that window.
Comment 7 Ian Neal 2012-07-16 07:48:05 PDT
(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.
Comment 8 Florian Quèze [:florian] [:flo] 2012-07-16 08:03:24 PDT
(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 :-(
Comment 9 Florian Quèze [:florian] [:flo] 2012-07-16 08:11:28 PDT
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 10 Florian Quèze [:florian] [:flo] 2012-07-16 08:12:44 PDT
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.
Comment 11 Florian Quèze [:florian] [:flo] 2012-07-16 08:45:17 PDT
Created attachment 642602 [details] [diff] [review]
Patch v3

Ian requested on IRC that I make the same changes for the suite/ file with r=him for suite/.
Comment 12 Florian Quèze [:florian] [:flo] 2012-07-16 08:53:00 PDT
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

Note You need to log in before you can comment on or make changes to this bug.