abuse of pluralform for addonsInstalled messages

RESOLVED FIXED in Firefox 59

Status

()

Firefox
General
RESOLVED FIXED
7 years ago
5 months ago

People

(Reporter: dynamis (Tomoya ASAI), Assigned: flod)

Tracking

({l12y})

unspecified
Firefox 59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
In short:
Pluralform should not be used single/multiple conditional branching.

Please define separate entity for single/multiple case for following messages to make it localizable.
http://mxr.mozilla.org/comm-central/source/mozilla/browser/locales/en-US/chrome/browser/browser.properties#43
addonsInstalled=#1 has been installed successfully.;#2 add-ons have been installed successfully.
addonsInstalledNeedsRestart=#1 will be installed after you restart #3.;#2 add-ons will be installed after you restart #3.

Details:
Add-on name is included when single add-on is installed.
Only a number of add-on is included when multiple add-ons are installed.

We should localize differently for single/multiple case but many locales (pluralrule=0,3,4...) cannot use different translation for single/multiple case translate this properly.
FYI: https://developer.mozilla.org/en/Localization_and_Plurals

So, please define separate entity for these messages.
(Reporter)

Updated

7 years ago
Keywords: l12y

Comment 1

7 years ago
Confirmed, but with an extra:

Keep the addonsInstalledNeedsRestart as is, but add a namedAddonInstalledRestart or so for the "we have one addon and want to show the name". ie, keep the plural magic for the numbers, because locales will still need that.
(Reporter)

Comment 2

7 years ago
Yes, entities like this is fine:

namedAddonsInstalled=#1 has been installed successfully.
addonsInstalled=An add-on has been installed successfully.;#2 add-ons have been installed successfully.
namedAddonsInstalledNeedsRestart=#1 will be installed after you restart #3.
addonsInstalledNeedsRestart=An add-on will be installed after you restart #3.;#2 add-ons will be installed after you restart #3.
(Assignee)

Updated

5 months ago
Assignee: nobody → francesco.lodolo
Comment hidden (mozreview-request)
(Assignee)

Comment 4

5 months ago
Comment on attachment 8934530 [details]
Bug 658191 - Fix plural form in Add-on installation messages

Does this approach make sense?
Attachment #8934530 - Flags: feedback?(l10n)

Updated

5 months ago
Attachment #8934530 - Flags: feedback?(l10n) → feedback+
(Assignee)

Updated

5 months ago
Attachment #8934530 - Flags: review?(aswan)
(Assignee)

Comment 5

5 months ago
@aswan
Checking the file's log, you're one the last persons to touch the file on the add-on part. Let me know if I should redirect the request elsewhere.

Comment 6

5 months ago
mozreview-review
Comment on attachment 8934530 [details]
Bug 658191 - Fix plural form in Add-on installation messages

https://reviewboard.mozilla.org/r/205436/#review211174

Hopefully we'll be getting rid of some of this code soon, but if this is causing trouble for translators lets get it fixed.

::: browser/base/content/browser-addons.js:454
(Diff revision 1)
> +          messageString = messageString.replace("#1", numAddons);
> +          messageString = messageString.replace("#2", brandShortName);

Any reason not to switch over to getFormattedString() here instead of this?  (same below)

::: browser/base/content/browser-addons.js:471
(Diff revision 1)
>            accessKey: gNavigatorBundle.getString("addonInstallRestartIgnoreButton.accesskey"),
>            callback: () => {},
>          }];
>        } else {
> -        messageString = gNavigatorBundle.getString("addonsInstalled");
> +        if (numAddons == 1) {
> +          messageString = gNavigatorBundle.getFormattedString("addonInstalled",

The second parameter (brandShortName) isn't used in this string
Attachment #8934530 - Flags: review?(aswan) → review+
(Assignee)

Comment 7

5 months ago
mozreview-review
Comment on attachment 8934530 [details]
Bug 658191 - Fix plural form in Add-on installation messages

https://reviewboard.mozilla.org/r/205436/#review211178

Yeah, unfortunately these strings are still creating a lot of confusion, so it's better to fix them.

I'm starting to look into errors in plural strings (wrong number of forms), and we have a lot of them on these.

::: browser/base/content/browser-addons.js:454
(Diff revision 1)
> +          messageString = messageString.replace("#1", numAddons);
> +          messageString = messageString.replace("#2", brandShortName);

The format for plural strings requires placeholders with #1, #2, etc. while getFormattedStrings uses printf placeholders (%S, %1$s).

::: browser/base/content/browser-addons.js:471
(Diff revision 1)
>            accessKey: gNavigatorBundle.getString("addonInstallRestartIgnoreButton.accesskey"),
>            callback: () => {},
>          }];
>        } else {
> -        messageString = gNavigatorBundle.getString("addonsInstalled");
> +        if (numAddons == 1) {
> +          messageString = gNavigatorBundle.getFormattedString("addonInstalled",

Ugh, too much copy and paste. Thanks for catching it.
Comment hidden (mozreview-request)

Comment 9

5 months ago
Pushed by francesco.lodolo@mozillaitalia.org:
https://hg.mozilla.org/integration/autoland/rev/b046a40bac95
Fix plural form in Add-on installation messages r=aswan

Comment 10

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b046a40bac95
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.