Remove remaining uses of `general.useragent.locale`

RESOLVED FIXED in Firefox 58

Status

()

Core
Internationalization
P3
normal
RESOLVED FIXED
a month ago
18 days ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

(Assignee)

Description

a month ago
We migrated almost all calls for "requested locale" to use LocaleService.

There are some leftover in the tests and one leftover in the code:
 - toolkit/components/search/nsSearchService.js

It's slightly tricky because it tests for if the value has been manually modified, which we do not provide as part of LocaleService API.

Anyhoo, we'll have to deal with it and liberate ourselves from `general.useragent.locale` once and for all :)
(Assignee)

Updated

a month ago
Blocks: 1346877
Priority: -- → P3
(Assignee)

Comment 1

a month ago
Jorg - no rush with that, but flagging you to put it on your radar. It should be always possible for you to use LocaleService setRequestedLocales/getRequestedLocales.
Flags: needinfo?(jorgk)
(Assignee)

Updated

a month ago
See Also: → bug 1410733
Blocks: 1410738
I filed bug 1410738 assuming that you're referring to the preference general.useragent.locale.

We have a few uses:
https://dxr.mozilla.org/comm-central/search?q=general.useragent.locale&redirect=true

These lines (four in TB/IB and one in SM)
  pref("general.useragent.locale", "@AB_CD@");
will just be removed since we won't be setting the preference any more, right?
Flags: needinfo?(jorgk)
(Assignee)

Comment 3

a month ago
> These lines (four in TB/IB and one in SM)
>  pref("general.useragent.locale", "@AB_CD@");
> will just be removed since we won't be setting the preference any more, right?

This is up in the air right now. We probably will start setting it at startup to multilocale.json once bug 1362617 lands. No rush with those. For now it's all about getting runtime code call the right API :)

Thank you!

Comment 4

29 days ago
The user-pref will need to stay for explicit UI language selection, I think.
(Assignee)

Comment 5

29 days ago
That's a good point.

The pref supports a single value, while we want a list. I'd like us to migrate to a new pref I believe that handles a list of values.

The end game result I'm after would:
1) get a list of fallback locales for a given locale at runtime (say: "it-IT, it, en-US" for "it" and "sr-Cyrl, ru-RU" for 'sr-Cyrl")
2) build a multilocale.json based on the locales we ask to package the build with and their deduplicated fallbacks (for example for "it-IT, sr-Cyrl" it would build a multilocale.json with ["it-IT, "it", "en-US", "sr-Cyrl", "ru-RU"])
3) package the 5 locales into the build and expose them at runtime so that L10nRegistry can register them
4) GetRequestedLocales should return either ["it-IT, it, en-US"] or ["sr-Cyrl", "ru-RU"]
5) SetRequestedLocales can change that
6) The UI for locale selection would show all 5 available languages at this point but when selected, it would set requestedLocales to a fallback chain for the selected locale.

That's two birds at once - fallback chains, and packaging with multilocales.

Let's start with just fallback chains:

1) Introduce some data structure in our tree that stores fallback chains maintained by l10n drivers
2) When repackaging, build the multilocale.json with said fallback chain
3) Introduce LocaleService::GetFallbackChainForLocale // "it-IT" => ["it-IT", "it", "en-US"]
4) introduce `intl.locale.requested-locales` pre-populated with the fallback chain

I'd like to leave the packaging multiple locales for later, just noting it here because it affects how I'm shaping the APIs.

Comment 6

29 days ago
I see multilocale.json to be a packaging optimization for L10nRegistry.

I would expect the fallback chains to be an independent dataset of what's part of our build.

I don't see where the actual default is coming from right now, in a single-locale German build, a Fennec build, or a linux distro build.
Comment hidden (mozreview-request)
(Assignee)

Updated

19 days ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
(Assignee)

Comment 8

19 days ago
This moves the remaining bits in our codebase to use LocaleService API.

The remaining uses are raw intl.properties, firefox-l10n.js, browser/app/profile/firefox.js etc.

Comment 9

18 days ago
mozreview-review
Comment on attachment 8924730 [details]
Bug 1410736 - Replace remaining uses of general.useragent.locale with LocaleService API.

https://reviewboard.mozilla.org/r/195954/#review201498

Looks reasonable to me, though I'm largely trusting that tryserver would tell us if it was badly broken!
Attachment #8924730 - Flags: review?(jfkthame) → review+
(Assignee)

Updated

18 days ago
Blocks: 1414390

Comment 10

18 days ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0637dca8a971
Replace remaining uses of general.useragent.locale with LocaleService API. r=jfkthame
Backed out for eslint failure at toolkit/components/extensions/test/xpcshell/test_ext_i18n_css.js:130: Unexpected var, use let or const instead:

https://hg.mozilla.org/integration/autoland/rev/fd9b6dde9feeec0c4b8406e1ea5920647878824d

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0637dca8a9718ca67b57fc879c8be0f22658452b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=142053692&repo=autoland
>  TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/xpcshell/test_ext_i18n_css.js:130:5 | Unexpected var, use let or const instead. (mozilla/var-only-at-top-level)
Flags: needinfo?(gandalf)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

18 days ago
Thanks for backing this out!
Flags: needinfo?(gandalf)

Comment 14

18 days ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f401d9f8a87d
Replace remaining uses of general.useragent.locale with LocaleService API. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/f401d9f8a87d
Status: ASSIGNED → RESOLVED
Last Resolved: 18 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.