The default bug view has changed. See this FAQ.

tabView2.title and tabview2.moveToUnnamedGroup.label need l10n comments (browser.properties)

RESOLVED FIXED in Firefox 16

Status

Firefox Graveyard
Panorama
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: flod, Assigned: raymondlee)

Tracking

Trunk
Firefox 16

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 629691 [details] [diff] [review]
v1
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Attachment #629691 - Flags: review?(ttaubert)
(Reporter)

Comment 2

5 years ago
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).
(Assignee)

Comment 3

5 years ago
(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".
(Reporter)

Comment 4

5 years ago
(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

5 years ago
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.
(Reporter)

Comment 6

5 years ago
I didn't think of that. If %2$S can be 1, we also need plural form. ("1 altra", "2+ altre").
(Assignee)

Comment 7

5 years ago
Created attachment 629716 [details] [diff] [review]
v2

Added plural form support
Attachment #629691 - Attachment is obsolete: true
Attachment #629691 - Flags: review?(ttaubert)
Attachment #629716 - Flags: review?(ttaubert)
(Assignee)

Comment 8

5 years ago
Created attachment 629717 [details] [diff] [review]
v2

Minor update
Attachment #629716 - Attachment is obsolete: true
Attachment #629716 - Flags: review?(ttaubert)
Attachment #629717 - Flags: review?(ttaubert)
(Reporter)

Comment 9

5 years ago
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.
(Assignee)

Updated

5 years ago
Attachment #629717 - Flags: review?(ttaubert)
(Assignee)

Comment 10

5 years ago
Created attachment 629829 [details] [diff] [review]
v3
Attachment #629717 - Attachment is obsolete: true
Attachment #629829 - Flags: review?(ttaubert)
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.
(Assignee)

Comment 12

5 years ago
Created attachment 629846 [details] [diff] [review]
v4
Attachment #629829 - Attachment is obsolete: true
Attachment #629829 - Flags: review?(ttaubert)
Attachment #629846 - Flags: review?(ttaubert)
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
(Assignee)

Comment 14

5 years ago
Created attachment 630177 [details] [diff] [review]
v5
Attachment #629846 - Attachment is obsolete: true
Attachment #629846 - Flags: review?(ttaubert)
Attachment #630177 - Flags: review?(dao)
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).
(Assignee)

Comment 16

5 years ago
(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.
(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.
(Assignee)

Comment 18

5 years ago
Created attachment 630206 [details] [diff] [review]
v6

I've found an example and followed hat.

http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.properties#177

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#605
Attachment #630177 - Attachment is obsolete: true
Attachment #630177 - Flags: review?(dao)
Attachment #630206 - Flags: review?(dao)
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.
Attachment #630206 - Flags: review?(dao) → review+

Comment 20

5 years ago
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.
(Assignee)

Comment 21

5 years ago
Created attachment 630476 [details] [diff] [review]
Patch for checkin

Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=87ee92c63181
(Assignee)

Comment 22

5 years ago
(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
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/f88227ac42f8
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 16
Should we merge this back into Aurora? Bug 588593 landed for Fx 15.

Comment 25

5 years ago
Let me ask in .l10n, this is actually an interesting border line case of breaking string freeze for the better of the web.
https://hg.mozilla.org/mozilla-central/rev/f88227ac42f8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]

Comment 27

5 years ago
Quorum in .l10n is to request this to get on aurora.
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
Attachment #630476 - Flags: approval-mozilla-aurora?
Comment on attachment 630476 [details] [diff] [review]
Patch for checkin

[Triage Comment]
Approving for Aurora 15 w/ string change, given Axel's comment.
Attachment #630476 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/edc666ffeeb2
status-firefox15: --- → fixed
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.