1617455 - Setting RequestedLocales policy to empty string changes intl.locale.requested to invalid value

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, enhancement, P3)

Description

[Sören Hentzschel]

Attachment: policies.json

STR:

1. use the attached policies.json file
2. start Firefox
3. open about:config and search for intl.locale.requested

According to https://github.com/mozilla/policy-templates#requestedlocales it should be allowed to use an empty string if you want it to use the operating system language.

Result:

The value for intl.locale.requested is "und".

This was reported to me by another user with Firefox ESR 68.3 on Windows a few weeks ago and I was able to reproduce it with Firefox ESR 68.5 on macOS Catalina.

Comment 1

[Mike Kaply [:mkaply]]

I am not seeing this with a fresh profile.

My guess is that intl.locale.requested had already been set by something else.

This begs the question as to whether or not we should clear intl.locale.requested if it has a user value when the policy is used.

zibi: is intl.locale.requested ever set by Firefox?

Comment 2

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

Nope. The only way customers should set requested locales is via SetRequestedLocales - https://searchfox.org/mozilla-central/search?q=symbol:_ZN17mozILocaleService19SetRequestedLocalesERK8nsTArrayI9nsTStringIcEE%2C_ZN7mozilla4intl13LocaleService19SetRequestedLocalesERK8nsTArrayI9nsTStringIcEE&redirect=false

and that doesn't seem to ever happen.

Comment 3

[Mike Kaply [:mkaply]]

I'm going to morph this bug into having policy reset the requested locales prefernces.

Side note, that und is actually not an invalid value. It's a standard value that means "undefined"

Comment 4

[Mike Kaply [:mkaply]]

zibi:

Looking at the code, I do see this:

https://searchfox.org/mozilla-esr68/rev/ab5ba64828155fe1018230030f0a4d75a5d01e47/intl/locale/LocaleService.cpp#767

There are cases where we still set the intl.locale.requested pref.

When I implemented policy, I was told that

Services.locale.requestedLocales ="";

Should cause Firefox to use the locale that corresponds to the operating system.

Is that still the case?

Could we be getting back und for the empty string case?

Comment 5

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

If you mean Services.locale.requestedLocales =[];, then it still works and sets the pref to empty which then hits https://searchfox.org/mozilla-esr68/rev/ab5ba64828155fe1018230030f0a4d75a5d01e47/intl/locale/LocaleService.cpp#107

Comment 6

[Mike Kaply [:mkaply]]

[Tracking Requested - why for this release]: If I can sneak this into the current ESR and Firefox, that would be great, but I understand we are in RC week.

This is a policy only bug, breaking some enterprise installs.

So this is my bug. I'm using .split and the array is coming back as [""] and I'm setting that as requestedLocales instead of []

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f84d098a59e
Use empty array for empty locale string. r=zbraniecki

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7f84d098a59e

Comment 10

[Mike Kaply [:mkaply]]

Comment on attachment 9130617 [details]
Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

Beta/Release Uplift Approval Request

• User impact if declined: Policy can't be used to set locale language
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Policy only, has automated test.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Policy fix
• User impact if declined: Policy can't be used to set locale language
• Fix Landed on Version: 75
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Policy only, has automated test.
• String or UUID changes made by this patch:

Comment 11

[Mike Kaply [:mkaply]]

I realize we're in RC week, but if this has a chance, it would be nice to have. It's blocking a couple people's rollouts.

It's last minute because it took me some time to recreate.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

This isn't going to make 74.0 at this point as the final release candidate build has already been created and signed off by QA, but we can take it for 68.6esr still. Per Mike, that's an acceptable outcome. Leaving 74 set to fix-optional as a possible ride-along in the event of a 74 dot release too.

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9130617 [details]
Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

We have already shipped 74RC2 and have no strong driver for a RC3 over the week end, sorry.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9130617 [details]
Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

Approved for 68.6esr. Leaving the release approval request for later consideration per the previous comment.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/f5d0f9a79276

Comment 16

[Pascal Chevrel:pascalc]

Comment on attachment 9130617 [details]
Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

Setting back the approval-mozilla-release flag, we are not taking it for 74 but if we have a (non-chemspill) dot release we could take it as a ridealong.

Comment 17

[Pascal Chevrel:pascalc]

Comment on attachment 9130617 [details]
Bug 1617455 - Use empty array for empty locale string. r?zbraniecki

No dot release planned for 74.