Wrong accesskey for abCardOverlay.dtd -> AIM.label

RESOLVED FIXED in Thunderbird 53.0

Status

defect
--
minor
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: artem, Assigned: bugzilla2007)

Tracking

Trunk
Thunderbird 53.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Thanks.  AIM field was added via bug 759328 and A, I, and M were assigned to other fields so apparently they picked R.  IRC field was added later by bug 792800, and it was decided to also assign R.  It's not unusual to have duplicate accesskeys - it just means you must sometimes hit the key twice to read the desired field.

What's more problematic that NEITHER field is focused by the access key alt+R. I don't know why - perhaps because it's a modal dialog?
Severity: normal → minor
Flags: needinfo?(acelists)
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #1)
> Thanks.  AIM field was added via bug 759328 and A, I, and M were assigned to
> other fields so apparently they picked R.  IRC field was added later by bug
> 792800, and it was decided to also assign R.  It's not unusual to have
> duplicate accesskeys - it just means you must sometimes hit the key twice to
> read the desired field.

Maybe that is not how it works? Or not on all platforms? As Alt+R does not work now.
Also notice in the code AIM has a lowercase "r" as accesskey, while IRC has uppercase "R".

> What's more problematic that NEITHER field is focused by the access key
> alt+R. I don't know why - perhaps because it's a modal dialog?

I can confirm that on Win XP. removing the "r" accesskey from AIM makes IRC (R) work again.

I tried different combinations, e.g. giving I to AIM and C to ICQ, but then C clashes with Contacts (top tabs row) and again C does not work. So it seem there must not be duplicate accesskeys.

So a working combination I could find is: M for AIM, N for MSN.
Would that be acceptable?
Flags: needinfo?(richard.marti)
Flags: needinfo?(florian)
Flags: needinfo?(acelists)
Version: unspecified → Trunk
It would be preferable to change only AIM, so that MSN keeps the starting M. I was thinking B for AIM because it's the next letter after A.  But if necessary determine why the R does not work for AIM.
(In reply to :aceman from comment #2)
> So a working combination I could find is: M for AIM, N for MSN.
> Would that be acceptable?

This would be the best solution. Using a not related letter is only the last solution.

And I would say this fields aren't used so much to make the accesskeys such important to not change.
Flags: needinfo?(richard.marti)
(In reply to :aceman from comment #2)

> So a working combination I could find is: M for AIM, N for MSN.
> Would that be acceptable?

That's fine with me. Also note that MSN is likely to die in the not so distant future (http://ismsndeadyet.com/) ; I'm not sure the field will be removed, but if it gets renamed to something like 'Skype web', the accesskey situation for it will be easier.
Flags: needinfo?(florian)
Aadithya, would you like to try this bug?
Flags: needinfo?(aadithyabk)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Artem, I don't see anything on this bug which justifies "wontfix" resolution; on the contrary, the problem persists (for [en-US] locale) and experienced contributors have already discussed and agreed on a solution. They asked somebody to fix it, and there was no response, and then this minor bug got out of sight. However, I don't see how that puts you in a position to just close this without further consultation or even a comment. If you are trying to express your dissatisfaction that this hasn't been fixed yet, let me tell you that this is NOT the right way to do that, as it is in violation of the rules guiding this platform (https://bugzilla.mozilla.org/page.cgi?id=etiquette.html).
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Fix it per comment 2, and subsequently approved by involved parties.
Assignee: nobody → bugzilla2007
Status: REOPENED → ASSIGNED
Attachment #8819569 - Flags: review?(richard.marti)
Status: ASSIGNED → NEW
Flags: needinfo?(aadithyabk)
Comment on attachment 8819569 [details] [diff] [review]
Fix accesskeys: AI_M_, MS_N_ --> I_R_C works again.

You don't need to change the entities because this is a en-US issue. Other locales have to use their own keys and check they are correct. For example de-DE uses other keys which don't overlap.

Without the entity change we can also uplift this bug to 52.
(In reply to Richard Marti (:Paenglab) from comment #9)
> Comment on attachment 8819569 [details] [diff] [review]
> Fix accesskeys: AI_M_, MS_N_ --> I_R_C works again.
> 
> You don't need to change the entities because this is a en-US issue. Other
> locales have to use their own keys and check they are correct. For example
> de-DE uses other keys which don't overlap.

I am aware of that and it's generally true, but in this case I deliberately decided to bump the entities anyway because I think that some other locales are very likely to face the same problem: Those proper names like Google Talk, AIM, Yahoo, Skype, etc. are unlikely to be translated, they are likely to be the same for a number of other translations. So I thought there's a high chance that they might have faced the same problem if they just copied the en-US strings and accesskeys, so bumping will be a reminder to check it out. Richard, what do you think?
Status: NEW → ASSIGNED
If we really want this for en-US only (after considering my comment 10), here's the reduced patch.

We could also fix it for TB52 for en-US only, and bump entities on Trunk to create attention.

Richard, pls decide, review, and land. Thanks.
Nitfix alignment
Attachment #8819638 - Attachment is obsolete: true
Comment on attachment 8819569 [details] [diff] [review]
Fix accesskeys: AI_M_, MS_N_ --> I_R_C works again.

Okay, for c-c to let check the localizers their accesskeys.
Attachment #8819569 - Flags: review?(richard.marti) → review+
Comment on attachment 8819640 [details] [diff] [review]
Patch V. 1.2: [en-US] Fix accesskeys: AI_M_, MS_N_ --> I_R_C works again.

And this one for TB 52. Then at least en-US is correct.

Please ask for approval when the other patch is in a Daily.
Attachment #8819640 - Flags: review+
https://hg.mozilla.org/comm-central/rev/cbd97df08042e6eee4a2823ade9dda017154bfa6

I checked-in with DONTBUILD to not start a whole build run for only string changes. This means, this change is only included in a Daily when a other patch without DONTBUILD lands. So today Daily will not include this change when no other important patch lands before.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
You need to log in before you can comment on or make changes to this bug.