329444 - messenger.xul l10n fixes

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Dwayne Bailey]

Some misnamed entities preventing good .label and .accesskey association

Comment 1

[Dwayne Bailey]

Attachment: Fixes .xul and .dtd files from mail/ and mailnews/

Comment 2

[Dwayne Bailey]

Any chances that this and the other Thunderbird related l10n fixes will ever get integrated?  We're updating those translations again and they're still not fixed, its sad as it creates the potential for mistakes around accelerator keys that we could be avoiding.

Comment 3

[Phil Ringnalda (:philor)]

Step one: assign the bug to yourself, so people will know you're working on it, and didn't just drop off a drive-by patch. Step two: ask for review, or absolutely positively nothing will happen.

I'll do for a reviewer, but first I'd like to hear what Axel has to say about whether it's a good idea to churn things that much for the gain, since my understanding of the needs and pains of localizers are mostly limited to the things I've already been yelled at for doing.

Comment 4

[Axel Hecht [:Pike]]

Generally, we should fix the issue with accesskeys and labels not matching.

Regarding this particular patch, I don't think that we should make the labels match Cmd, as commands are actions and don't really have UI, thus no accesskeys. They could have command keys.

So, in this particular case, I'd prefer to get the accesskey parts fixed instead of the labels. Semantic description is key here, which is why it matters that the commands itself just don't have one UI.

Similar arguments may or may not hold for other bugs bocking bug 329432, but you may want to run this by a more format discussion. mozilla.dev.tech.xul sounds like the right group for this discussion.

Comment 5

[Dwayne Bailey]

(In reply to comment #4)
> Generally, we should fix the issue with accesskeys and labels not matching.

Yes please.  Without them matching localisers have no idea what accesskey belongs to which label.  Our tools automate that and also can't guess but with some of the misalignment not even a human can guess without having to trace the XUL files and check. There where also cases of reuse of labels which doesn't work for all languages.

> Regarding this particular patch, I don't think that we should make the labels
> match Cmd, as commands are actions and don't really have UI, thus no
> accesskeys. They could have command keys.

I think this was actually the messiest file.  My decision was to look at what was the most prevalent form and what would create the smallest set of changes.  Thus often you'd have .label, .accesskey and .key so I chose to align on whatever was most common.  I spent quite a bit of time double checking against the XUL files, but that was long ago.

> So, in this particular case, I'd prefer to get the accesskey parts fixed
> instead of the labels. Semantic description is key here, which is why it
> matters that the commands itself just don't have one UI.

I'm happy to revise this one based on that input as I said my decision was based on minimal changes as I myself was not sure of the hidden meaning within the main part of the key.

> Similar arguments may or may not hold for other bugs bocking bug 329432, but
> you may want to run this by a more format discussion. mozilla.dev.tech.xul
> sounds like the right group for this discussion.

Here is my approach:

1) There are many obvious and easy ones.  I will try go through those and follow the suggestions made by Phil.  So we can close those before raising any large discussion.  PS Phil, your process suggestion to me is amazingly odd, I'm not sure how i was meant to sniff that out of the ether.  But now that I know it I will follow it.

2) I'll then go through the remaining patches that do *Cmd magic and try adjust those.

3) I think before even beginning discussion on another mailing list :)  I
ll start a page, unless one already exists on XUL naming conventions for localisable strings.  From there input and discussion can go forward.

Comment 6

[Phil Ringnalda (:philor)]

(In reply to comment #5)
> PS Phil, your process suggestion to me is amazingly odd, I'm
> not sure how i was meant to sniff that out of the ether.

Well, that's "minor, and not everyone cares, but I think it's useful, Step 1" and "absolutely required by every single patch to Mozilla, other than l10n before a freeze, Step 2." Sniffing out the actual current steps, separate from the outdated information, is fairly difficult, but there's http://www.mozilla.org/hacking/life-cycle.html and http://www.mozilla.org/hacking/reviewers.html and http://www.mozilla.org/owners.html and http://www.mozilla.org/projects/thunderbird/review.html (though out of all of those, life-cycle's "Ask on IRC" (#l10n, or at some times of the day #maildev, will work as well as #developers) is probably the single best bit of advice).

And, sadly, owners.html (and "Ask on IRC") are going to be useful to you, because what I meant was "I'll do for a reviewer for this patch, to files in mail/ only, because I'm a Thunderbird peer and can review code there" - the other patches are mostly to code shared with SeaMonkey, or SeaMonkey and Firefox, where you'll need a reviewer from that module and generally superreview.

Comment 7

[Axel Hecht [:Pike]]

The best place to put further coding style comments would be http://developer.mozilla.org/en/docs/Talk:Writing_localizable_code. The existing comments haven't been merged into the main page yet, but if we get a concise listing, that should be fairly easy.

Comment 8

[Vlado Valastiak [:wladow]]

Attachment: patch against hg

Notes:
- per comment 4 entity nextUnreadThreadCmd renamed to nextUnreadThread
- several accesskeys in original patch are already fixed

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 338455 [details] [diff] [review]
patch against hg

I'm just useless today ;)

I'm almost out the door on vacation, but Magnus isn't (I hope).

Comment 10

[[e-mail]]

Comment on attachment 338455 [details] [diff] [review]
patch against hg

>@@ -311,8 +311,8 @@
> <!ENTITY nextFlaggedMsgCmd.accesskey "F">
> <!ENTITY nextUnflaggedMsgCmd.label "Unflagged Message">
> <!ENTITY nextUnreadThread.label "Unread Thread">
>-<!ENTITY nextUnreadThreadCmd.accesskey "T">
>-<!ENTITY nextUnreadThreadCmd.key "t">
>+<!ENTITY nextUnreadThread.accesskey "T">
>+<!ENTITY nextUnreadThread.key "t">
> <!ENTITY prevMenu.label "Previous">
> <!ENTITY prevMenu.accesskey "P">
> <!ENTITY prevMsgCmd.label "Message">
>@@ -326,7 +326,7 @@
> <!ENTITY goForwardCmd.label "Forward">
> <!ENTITY goForwardCmd.commandKey "]">
> <!ENTITY prevFlaggedMsgCmd.label "Flagged Message">
>-<!ENTITY previousFlaggedMsgCmd.accesskey "F">
>+<!ENTITY prevFlaggedMsgCmd.accesskey "F">
> <!ENTITY backCmd.label ".Back">
> <!ENTITY backCmd.accesskey "B">
> <!ENTITY forwardCmd.label ".Forward">
Looking at these entities I'd say most of them end in Cmd so for consistency I think it would be better to keep nextUnreadThreadCmd too.

Comment 11

[Vlado Valastiak [:wladow]]

OK, let's see what Magnus thinks about it and I'll make a new patch for checkin, if there's a consensus.

Comment 12

[[e-mail]]

Comment on attachment 338455 [details] [diff] [review]
patch against hg

Oops, sorry for not reading comment #4 first. (But I'd still prefer consistency.)

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 338455 [details] [diff] [review]
patch against hg

>+          <menuitem id="menu_compactFolder" label="&compactFolders.label;" accesskey="&compactFolders.accesskey;" command="cmd_compactFolder"/>

Please break the overlong line while you're touching it, align next row with id.


>+              <menuitem id="menu_nextUnreadThread" label="&nextUnreadThread.label;" accesskey="&nextUnreadThread.accesskey;" command="cmd_nextUnreadThread"

Here too.

r=mkmelin with that

Comment 14

[Vlado Valastiak [:wladow]]

Attachment: for checkin

Comment 15

[Phil Ringnalda (:philor)]

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