1635561 - Clean up MozIntl

Status: RESOLVED FIXED

(Core :: Internationalization, task, P3)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

With the flock of ECMA402 updates landing in SpiderMonkey(*), we should be able to clean up and reduce the scope of MozIntl quite substantially.

This should lead to perf wins due to bug 1504168 and related.

*) bug 1569103, bug 1557718, bug 1589095, bug 1632434

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

One thing that is left is locale management, and I'd like to explore an option to move locale management used in mozIntl to https://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#117-141 - we should just chose the right locale fallback chain for JS environment separately for chrome/non-chrome and then

new Intl.DateTimeformat() in chrome process should return the Firefox UI locale, while in web process should return the web exposed locale chain. This would remove a bit part of mozIntl.

Comment 2

[Dan Minor [:dminor]]

Having looked at this and Bug 1639515, I think it makes sense to do the cleanup Zibi suggested in Comment 1 first, and then rebase and land Bug 1639515 afterwards. I can pick up a few of the changes unrelated to DisplayNames from 1639515 as part of this bug.

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1635561 - Use RegionalPrefLocales rather than AppLocale in xpc_LocalizeRuntime; r=zbraniecki!

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1635561 - Remove unnecessary wrappers around Intl in MozIntl; r=zbraniecki!

Not that we use RegionalPrefLocales rather than AppLocale in
xpc_LocalizeRuntime, the wrappers in MozIntl to set this up are no longer
needed.

Depends on D98390

Comment 5

[Dan Minor [:dminor]]

Attachment: Bug 1635561 - Add ListFormat to MozIntl; r=zbraniecki!

Keeping MozIntl in sync with Intl will allow people to use MozIntl consistently
until we're ready to remove it completely. This is based on work done by André
Bargull for Bug 1639515.

Depends on D98391

Comment 6

[Dan Minor [:dminor]]

Attachment: Bug 1635561 - Add Locale to MozIntl; r=zbraniecki!

This adds Locale to MozIntl and uses it to replaces some instances where we
used regular expressions to parse language tags. It is based on work done by
André Bargull in Bug 1639515.

Depends on D98392

Comment 7

[Dan Minor [:dminor]]

Try run here: https://treeherder.mozilla.org/jobs?repo=try&revision=409744cd18ebe57bcd80fe806ce38308b3ed2e9c

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05b8c9d0b50d
Use RegionalPrefLocales rather than AppLocale in xpc_LocalizeRuntime; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/562e4afa8a10
Remove unnecessary wrappers around Intl in MozIntl; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/21d942cfc35d
Add ListFormat to MozIntl; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/d4ef63a42094
Add Locale to MozIntl; r=zbraniecki

Comment 9

[Razvan Maries]

Backed out for perma failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=os%2Cx%2C10.14%2Cwebrender%2Copt%2Cmochitests%2Cwithout%2Ce10s%2Ctest-macosx1014-64-qr%2Fopt-mochitest-chrome-1proc%2Cc2&revision=d4ef63a4209417db9d840a576cfa341e9a7adfd9&selectedTaskRun=aoXUpP88R7qOllhKs0jC5A.0

Log: https://treeherder.mozilla.org/logviewer?job_id=323483011&repo=autoland&lineNumber=4671

Backout: https://hg.mozilla.org/integration/autoland/rev/6b65a4a632e9d24bb048fdcb261f7777d032cc0b

Comment 10

[Dan Minor [:dminor]]

I think the problem is that with these changes [1] we use the first item in the regionalPrefsLocales array but with the getLocales wrapper in mozIntl.jsm, we use the full array [2]. I think this makes a difference when we attempt to resolve the locale at [3].

The test that is failing calls localeService.negotiateLanguages [4] to determine the default locale if no locales are specified. I think we need to either change the code in XPCLocale.cpp to also call NegotiateLanguages, or change the test expectations to not do so. I'm guessing we'd want to add the call to NegotiateLanguages in XPCLocale but I'm not completely sure.

Zibi, what do you think?

[1] https://hg.mozilla.org/integration/autoland/rev/05b8c9d0b50d#l1.21
[2] https://searchfox.org/mozilla-central/source/toolkit/components/mozintl/mozIntl.jsm#26
[3] https://searchfox.org/mozilla-central/source/js/src/builtin/intl/IntlObject.js#209
[4] https://searchfox.org/mozilla-central/source/dom/tests/mochitest/chrome/test_intlUtils_getLocaleInfo.html#21

Comment 11

[Dan Minor [:dminor]]

I dug into this a little deeper, and I was completely wrong about the array stuff, I only have one entry on my system anyway.

As far as I can tell, the difference is the call to NegotiateLanguages. On my system, my regionalPrefsLocale is "en-CA" and my availableLocales is "en-US", and we end up using "en-US". Prior to my changes, we used the app locale in XPCLocale, which is also "en-US" and the test passed. With my changes, we now use the regionalPrefsLocale in XPCLocale, which is "en-CA", and the test fails.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee7bed950965
Use RegionalPrefLocales rather than AppLocale in xpc_LocalizeRuntime; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/36bff8882d55
Remove unnecessary wrappers around Intl in MozIntl; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/fa5240689523
Add ListFormat to MozIntl; r=zbraniecki
https://hg.mozilla.org/integration/autoland/rev/47eb8c778c41
Add Locale to MozIntl; r=zbraniecki

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ee7bed950965
https://hg.mozilla.org/mozilla-central/rev/36bff8882d55
https://hg.mozilla.org/mozilla-central/rev/fa5240689523
https://hg.mozilla.org/mozilla-central/rev/47eb8c778c41