1301933 - Investigate using English as the default plural form for unlocalized strings

Status: REOPENED

(Localization Infrastructure and Tools :: General, defect)

Description

[magicp]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160910030426

Steps to reproduce:

1. Start Nightly in Arabic with new profile  (*don't use Force RTL with en-US for reproduce)
2. Go to about:home
3. Open Browser Console (Ctrl+Shift+J)
4. Open DevTools > Toolbox Options > Check on all devtools
5. Switch devtool tab from Inspector to DOM


Actual results:

When open Debugger, Style Editor and Scratchpad, the following logs are in Browser Console.

[Debugger]  * (2): message-repeats
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 3 is invalid -- plural rule #12;  (2) plural-form.js:191

[Style Editor]
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 3 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 4 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 5 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 6 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 7 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 8 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 9 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 10 is invalid -- plural rule #12;  (2) plural-form.js:191

[Scratchpad]
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 3 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 4 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 5 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 6 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 7 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 8 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 9 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #2 of '#1 repeat;#1 repeats' for value 10 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #3 of '#1 repeat;#1 repeats' for value 11 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #3 of '#1 repeat;#1 repeats' for value 12 is invalid -- plural rule #12;  (2) plural-form.js:191
plural-form.js: Index #3 of '#1 repeat;#1 repeats' for value 13 is invalid -- plural rule #12;  (2) plural-form.js:191



Expected results:

These logs is intentional ? If error, fix it.

Comment 1

[Julian Descottes [:jdescottes]]

I can reproduce this behavior on Firefox 50 as well, are you sure about your regression window?

Furthermore, I think these logs are intentional and helpful for localization teams. They indicate that the string "#1 repeat;#1 repeats" is used for plural localization, but does not support all the plural forms needed by the current locale (in this case the string was simply not localized).

So IMO the logs are fine and this is not a regression. Am I missing something?

Comment 2

[magicp]

(In reply to Julian Descottes [:jdescottes] from comment #1)
> I can reproduce this behavior on Firefox 50 as well, are you sure about your
> regression window?
> 
> Furthermore, I think these logs are intentional and helpful for localization
> teams. They indicate that the string "#1 repeat;#1 repeats" is used for
> plural localization, but does not support all the plural forms needed by the
> current locale (in this case the string was simply not localized).

I cannot confirm this regression window in Arabic by Mozregression-gui. So I confimed plural-form.js in Mercurial. Also I cannot reproduce this bug before that.
https://hg.mozilla.org/mozilla-central/log?rev=plural-form.js

> So IMO the logs are fine and this is not a regression. Am I missing
> something?

I don't know.

Comment 3

[Julian Descottes [:jdescottes]]

I reproduced the same behavior with https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-aurora-l10n/firefox-50.0a2.ar.mac.dmg , which doesn't use plural-form.js, but PluralForm.jsm (plural-form.js is simply a copy of PluralForm.jsm, cleaned up of Chrome dependencies). I also tried with 49 (https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-beta-l10n/firefox-49.0.ar.mac.dmg) and again same behavior.

I checked in the history of http://hg.mozilla.org/l10n-central/ar/file/tip/devtools/client/webconsole.properties , and it's never been localized, so I think this error has always been logged in the ar locale.

Closing as invalid.

Comment 4

[magicp]

Hi Julian, Thanks for your investigation. Do localization teams need to work about this?

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to magicp from comment #4)
> Hi Julian, Thanks for your investigation. Do localization teams need to work
> about this?

This string is used as the tooltip of the "repeat" bubble in the webconsole. I wouldn't say it's the most important string to get translated, but since this is a pluralized string, it can lead to nonsensical strings in locales with more than 2 plural forms.

Reopening and updating the bug subject, but I'm not sure localization teams are using bugzilla to track missing strings. Let's ask.

Comment 6

[Julian Descottes [:jdescottes]]

Francesco: this bug is about a missing translation for one of the webconsole strings. As this is a string using plural forms, it might be nice to prioritize its localization. As of today in arabic, if you this string is pluralized for more than N>2 occurences, the message displayed is "N repeat" (which is even incorrect in english).

As I said above, not sure how missing localizations should be tracked. If bugzilla is fine, could you CC someone working on the Arabic locale?

Comment 7

[Francesco Lodolo [:flod]]

There's no such thing as prioritizing missing strings: locales are aware of the missing strings in their tools, and in the l10n dashboards, but there's no priority. We also don't file bugs for missing strings. 

For some locales, devtools are a low priority though, meaning that they'll focus on other missing parts first, and some will just use English. This means we'll have plenty of these warnings if a locale uses 3 plural forms but falls back to English, which has two.

IMO the bug remains INVALID, unless you want to use it to reduce the log level of that function.

> Francesco: this bug is about a missing translation for one of the webconsole
> strings. As this is a string using plural forms, it might be nice to
> prioritize its localization. As of today in arabic, if you this string is
> pluralized for more than N>2 occurences, the message displayed is "N repeat"
> (which is even incorrect in english).

I don't think I'm getting this last part: are you saying the plural form fails completely?

Comment 8

[Julian Descottes [:jdescottes]]

(In reply to Francesco Lodolo [:flod] from comment #7)
> There's no such thing as prioritizing missing strings: locales are aware of
> the missing strings in their tools, and in the l10n dashboards, but there's
> no priority. We also don't file bugs for missing strings. 
> 
> For some locales, devtools are a low priority though, meaning that they'll
> focus on other missing parts first, and some will just use English. This
> means we'll have plenty of these warnings if a locale uses 3 plural forms
> but falls back to English, which has two.

It doesn't really fall back to english (see below). 

> 
> IMO the bug remains INVALID, unless you want to use it to reduce the log
> level of that function.
> 

I mentioned "error" but it's actually just a log in the browser console right now. Can clutter a bit the output though...

> > Francesco: this bug is about a missing translation for one of the webconsole
> > strings. As this is a string using plural forms, it might be nice to
> > prioritize its localization. As of today in arabic, if you this string is
> > pluralized for more than N>2 occurences, the message displayed is "N repeat"
> > (which is even incorrect in english).
> 
> I don't think I'm getting this last part: are you saying the plural form
> fails completely?

It fails as expected I would say :)

Arabic has 6 plural forms, the string to pluralize here is '#1 repeat;#1 repeats'. If the plural form that should be used is the 3rd, 4th, 5th or 6th, plural-form.js will see it only has 2 plural forms in this string and will default to the first one. 
=> Which means that '#1 repeat;#1 repeats' for 3 occurrences is pluralized to '3 repeat' instead of '3 repeats'. 

I suppose that's why there is a log for this, this makes it slightly more important to localize pluralized strings than regular strings.

We need to know that the string is not localized to fallback to english. I suppose we can fallback to english if there is a mismatch between the plural forms expected by the locale and the ones available in the string. We could then get rid of the log, since an unlocalized plural string would simply properly fallback to english as expected. How does that sound?

Comment 9

[Francesco Lodolo [:flod]]

It does fallback to English, the problem is that pluralforms can't know if the string it sees is Arabic or English, and assumes Arabic. 

Then tries to find a 3rd plural form, which is not available in English, and fails, falling back to the first form
https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/intl/locale/PluralForm.jsm#140

When a string is missing from the file, compare-locales add it at build time, otherwise the build would be usable (YSOD in XUL).

Comment 10

[Axel Hecht [:Pike]]

This is a known shortcoming of our legacy l10n backend.

In l20n, a missing string falls back to getting the string from the fallback language context, which has the corresponding plural form, and doesn't show this problem.

That's the right fix, and implementing it corresponds to implementing l20n in large extents, so there's no "backporting" of that functionality, really.

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Axel Hecht [:Pike] from comment #10)
> That's the right fix, and implementing it corresponds to implementing l20n
> in large extents, so there's no "backporting" of that functionality, really.

I agree that's the proper long-term fix, but wouldn't loading the en-US rule (1) and pluralize the string be enough as a patch in case I'm trying to load a plural form that is not available?

Comment 12

[Axel Hecht [:Pike]]

There's also the error scenario where the localizer doesn't give the right number of options in a localized string.

It's an interesting area of research to see which form to fall back to is best across the error scenarios of "bad count of localized entries" and "fallback to en-US". It'd be nice to hit "Other" in CLDR lingo as often as possible, I guess.

Comment 13

[Julian Descottes [:jdescottes]]

Repurposing the bug to discuss the possibility of using english as the default plural form if a mismatch between the locale's expected plural forms and a localized string is detected. 

Moving to Localization Infrastructure, hope this is OK.

Unassigning myself for now. I will upload my current patches here for reference. Reading the last comment I understand you want to gather data before changing this, which I won't have time for at the moment.

Comment 14

[Julian Descottes [:jdescottes]]

Attachment: Bug 1301933 - Use english as the default plural form in PluralForm.jsm

Review commit: https://reviewboard.mozilla.org/r/78654/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78654/

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1301933 - update devtools plural-form.js after change in original PluralForm.jsm

Review commit: https://reviewboard.mozilla.org/r/78656/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78656/