Last Comment Bug 760794 - tabView2.title and tabview2.moveToUnnamedGroup.label need l10n comments (browser.properties)
: tabView2.title and tabview2.moveToUnnamedGroup.label need l10n comments (brow...
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 16
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on:
Blocks: 588593
  Show dependency treegraph
 
Reported: 2012-06-02 05:31 PDT by Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3
Modified: 2016-04-12 14:00 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (1.33 KB, patch)
2012-06-03 21:40 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v2 (2.45 KB, patch)
2012-06-04 00:47 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v2 (2.64 KB, patch)
2012-06-04 00:52 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v3 (2.55 KB, patch)
2012-06-04 09:53 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v4 (3.27 KB, patch)
2012-06-04 10:41 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v5 (3.27 KB, patch)
2012-06-05 08:32 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review
v6 (3.35 KB, patch)
2012-06-05 10:03 PDT, Raymond Lee [:raymondlee]
dao+bmo: review+
Details | Diff | Review
Patch for checkin (3.66 KB, patch)
2012-06-06 01:52 PDT, Raymond Lee [:raymondlee]
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-02 05:31:53 PDT
http://hg.mozilla.org/mozilla-central/rev/1c5f48f57ef1

tabview2.moveToUnnamedGroup.label=%S and %S more

Needs a l10n comment explaining what those "%S" stand for.

Since we are in the process, tabView2.title should be fixed too.
Comment 1 Raymond Lee [:raymondlee] 2012-06-03 21:40:23 PDT
Created attachment 629691 [details] [diff] [review]
v1
Comment 2 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-03 22:24:51 PDT
I don't think it's wise to change from
tabview2.moveToUnnamedGroup.label=%S and %S more
to
tabview2.moveToUnnamedGroup.label=%1$S and %2$S more
without changing the key name (CCing Axel, this would probably raise a warning in compare-locales).
Comment 3 Raymond Lee [:raymondlee] 2012-06-03 22:39:03 PDT
(In reply to Francesco Lodolo [:flod] from comment #2)
> I don't think it's wise to change from
> tabview2.moveToUnnamedGroup.label=%S and %S more
> to
> tabview2.moveToUnnamedGroup.label=%1$S and %2$S more
> without changing the key name (CCing Axel, this would probably raise a
> warning in compare-locales).

The reason I changed that is because it's possible that in some languages, the structure order might be "%2$S foo %1$S bar".
Comment 4 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-03 22:46:00 PDT
(In reply to Raymond Lee [:raymondlee] from comment #3)
> The reason I changed that is because it's possible that in some languages,
> the structure order might be "%2$S foo %1$S bar".

I absolutely agree, in fact I'm not questioning the change, but if this can be done without changing also the key name ;-)
Comment 5 Axel Hecht [:Pike] 2012-06-03 23:19:16 PDT
Changing implicit order to explicit order is fine without key change. The semantics don't change, they just get more crisp.

The question I have is whether "foo and 15 more" needs to use plural forms for the second half. CC more community, in particular Polish and Japanese as spot checks.
Comment 6 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-03 23:22:03 PDT
I didn't think of that. If %2$S can be 1, we also need plural form. ("1 altra", "2+ altre").
Comment 7 Raymond Lee [:raymondlee] 2012-06-04 00:47:37 PDT
Created attachment 629716 [details] [diff] [review]
v2

Added plural form support
Comment 8 Raymond Lee [:raymondlee] 2012-06-04 00:52:01 PDT
Created attachment 629717 [details] [diff] [review]
v2

Minor update
Comment 9 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-04 00:58:26 PDT
I don't think that's the right way to support plural forms (but I'm not a developer)
https://developer.mozilla.org/en/Localization_and_Plurals#Developing_with_PluralForm

BTW, that new string will need a new key name for sure.
Comment 10 Raymond Lee [:raymondlee] 2012-06-04 09:53:23 PDT
Created attachment 629829 [details] [diff] [review]
v3
Comment 11 Dão Gottwald [:dao] 2012-06-04 10:13:42 PDT
Comment on attachment 629829 [details] [diff] [review]
v3

"UnnamedGroupAndMore" doesn't make sense, since "more" refers to tabs...
Also, drop the "2" in tabview2 when changing the string name.
Comment 12 Raymond Lee [:raymondlee] 2012-06-04 10:41:53 PDT
Created attachment 629846 [details] [diff] [review]
v4
Comment 13 Dão Gottwald [:dao] 2012-06-05 02:01:24 PDT
Comment on attachment 629846 [details] [diff] [review]
v4

>+tabView.title=%S - Group Your Tabs
>+tabview.moveToUnnamedGroup.label=%1$S and 1 more;%1$S and #1 more

either tabview or tabView for both strings
Comment 14 Raymond Lee [:raymondlee] 2012-06-05 08:32:11 PDT
Created attachment 630177 [details] [diff] [review]
v5
Comment 15 Dão Gottwald [:dao] 2012-06-05 08:45:27 PDT
Comment on attachment 630177 [details] [diff] [review]
v5

>-tabView2.title=%S - Group Your Tabs
>-tabview2.moveToUnnamedGroup.label=%S and %S more
>+# LOCALIZATION NOTE (tabview.title): %S is the application name.
>+# LOCALIZATION NOTE (tabview.moveToUnnamedGroup.label): 
>+# %1$S is the page title of the first tab in the unnamed group, 
>+# #1 is the number of remaining tabs
>+tabview.title=%S - Group Your Tabs
>+tabview.moveToUnnamedGroup.label=%1$S and 1 more;%1$S and #1 more

Use %S instead of %1$S, since you removed the second %S.

You should probably note that tabview.moveToUnnamedGroup.label uses the plural form (http://mxr.mozilla.org/mozilla-central/search?string=http%3A%2F%2Fdeveloper.mozilla.org%2Fen%2Fdocs%2FLocalization_and_Plurals).
Comment 16 Raymond Lee [:raymondlee] 2012-06-05 09:01:42 PDT
(In reply to Dão Gottwald [:dao] from comment #15)
> Comment on attachment 630177 [details] [diff] [review]
> v5
> 
> >-tabView2.title=%S - Group Your Tabs
> >-tabview2.moveToUnnamedGroup.label=%S and %S more
> >+# LOCALIZATION NOTE (tabview.title): %S is the application name.
> >+# LOCALIZATION NOTE (tabview.moveToUnnamedGroup.label): 
> >+# %1$S is the page title of the first tab in the unnamed group, 
> >+# #1 is the number of remaining tabs
> >+tabview.title=%S - Group Your Tabs
> >+tabview.moveToUnnamedGroup.label=%1$S and 1 more;%1$S and #1 more
> 
> Use %S instead of %1$S, since you removed the second %S.

If I use %S instead of %1$S, I would have to pass the same string twice to getFormattedString() as below.  Do you think it's better to use %1$S?

gNavigatorBundle.getFormattedString("tabview.moveToUnnamedGroup.label", [topChildLabel, topChildLabel]);

> 
> You should probably note that tabview.moveToUnnamedGroup.label uses the
> plural form
> (http://mxr.mozilla.org/mozilla-central/search?string=http%3A%2F%2Fdeveloper.
> mozilla.org%2Fen%2Fdocs%2FLocalization_and_Plurals).

Yes, we want to use plural form.
Comment 17 Dão Gottwald [:dao] 2012-06-05 09:13:58 PDT
(In reply to Raymond Lee [:raymondlee] from comment #16)
> If I use %S instead of %1$S, I would have to pass the same string twice to
> getFormattedString() as below.  Do you think it's better to use %1$S?
> 
> gNavigatorBundle.getFormattedString("tabview.moveToUnnamedGroup.label",
> [topChildLabel, topChildLabel]);

I see. Do we have a precedent for this?

> > You should probably note that tabview.moveToUnnamedGroup.label uses the
> > plural form
> > (http://mxr.mozilla.org/mozilla-central/search?string=http%3A%2F%2Fdeveloper.
> > mozilla.org%2Fen%2Fdocs%2FLocalization_and_Plurals).
> 
> Yes, we want to use plural form.

You should add it to the localization note.
Comment 19 Dão Gottwald [:dao] 2012-06-05 10:16:59 PDT
Comment on attachment 630206 [details] [diff] [review]
v6

>+# #1 the page title of the first tab in the unnamed group, 
>+# #2 the number of remaining tabs

# #1 is the page title of the first tab in the unnamed group,
# #2 is the number of remaining tabs.
Comment 20 Axel Hecht [:Pike] 2012-06-05 11:40:40 PDT
Comment on attachment 630206 [details] [diff] [review]
v6

Review of attachment 630206 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/browser.properties
@@ +310,5 @@
> +# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> +# #1 the page title of the first tab in the unnamed group, 
> +# #2 the number of remaining tabs
> +tabview.title=%S - Group Your Tabs
> +tabview.moveToUnnamedGroup.label=#1 and 1 more;#1 and #2 more

Please put the comments on top of the entries that they're for, see also my previous comment.
Comment 21 Raymond Lee [:raymondlee] 2012-06-06 01:52:58 PDT
Created attachment 630476 [details] [diff] [review]
Patch for checkin

Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=87ee92c63181
Comment 22 Raymond Lee [:raymondlee] 2012-06-06 09:00:40 PDT
(In reply to Raymond Lee [:raymondlee] from comment #21)
> Created attachment 630476 [details] [diff] [review]
> Patch for checkin
> 
> Pushed to try and waiting for results
> https://tbpl.mozilla.org/?tree=Try&rev=87ee92c63181

Passed try
Comment 23 Tim Taubert [:ttaubert] 2012-06-06 09:18:40 PDT
https://hg.mozilla.org/integration/fx-team/rev/f88227ac42f8
Comment 24 Tim Taubert [:ttaubert] 2012-06-06 09:19:51 PDT
Should we merge this back into Aurora? Bug 588593 landed for Fx 15.
Comment 25 Axel Hecht [:Pike] 2012-06-06 10:12:45 PDT
Let me ask in .l10n, this is actually an interesting border line case of breaking string freeze for the better of the web.
Comment 26 Tim Taubert [:ttaubert] 2012-06-09 08:36:11 PDT
https://hg.mozilla.org/mozilla-central/rev/f88227ac42f8
Comment 27 Axel Hecht [:Pike] 2012-06-09 10:34:19 PDT
Quorum in .l10n is to request this to get on aurora.
Comment 28 Tim Taubert [:ttaubert] 2012-06-10 02:12:46 PDT
Comment on attachment 630476 [details] [diff] [review]
Patch for checkin

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 588593
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: Yes, please see comment #27
Comment 29 Alex Keybl [:akeybl] 2012-06-11 13:41:33 PDT
Comment on attachment 630476 [details] [diff] [review]
Patch for checkin

[Triage Comment]
Approving for Aurora 15 w/ string change, given Axel's comment.
Comment 30 Tim Taubert [:ttaubert] 2012-06-14 06:27:30 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/edc666ffeeb2

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