1450270 - Switch away from general.useragent.locale broke all Acer language specific preloads

Status: VERIFIED FIXED

(Firefox :: Distributions, defect, P1)

Description

[Mike Kaply [:mkaply]]

The switch to intl.locale.requested from general.useragent.locale broke all non-US Acer builds.

The reason is that they set the general.useragent.locale preference as a default preference and the migration code only handles user preferences.

They ship all the locales and then set the default in distribution.ini.

I'm going to add a workaround to distribution.js for this.

Comment 1

[Mike Kaply [:mkaply]]

[Tracking Requested - why for this release]: If we do a chemspill, we need this. It's a huge impact for our partner.

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Bug 1450270 - Special case general.useragent.locale in distributions.

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

Comment 3

[Mike Kaply [:mkaply]]

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232786/diff/1-2/

Comment 4

[Mike Kaply [:mkaply]]

Come to find out they are setting it in the [Preferences] section of distribution.ini which makes things much easier.

Comment 5

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Sounds like this has a pretty broad user impact. Tracking as a blocker for 59 (meaning that we will consider 59 dot release for it).   

How did we (or Acer) miss this issue in QA?  And, how will we know through testing that we've got it right in the fix?

Comment 6

[Mike Kaply [:mkaply]]

> How did we (or Acer) miss this issue in QA?

I can't speak for Acer, but I don't think the l10n team expected the general.useragent.locale to be a default pref, only a user set pref. So the patch they did didn't account for it.

The Acer stuff was before I was at Mozilla, so I didn't know about their specific scenario.

> And, how will we know through testing that we've got it right in the fix?

I have sample distribution.ini files from Acer for the failing case so I know exactly what was happening and why.

I'll investigate if I can create an automated test for it.

Comment 7

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

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

https://reviewboard.mozilla.org/r/232786/#review238262

This should work, although for the record - `intl.locale.requested` has different characteristics than `general.useragent.locale` had.

In particular it can take one of three types of values:

https://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#105

which is something the old pref was not designed to handle.

In particular, I'm wondering if setting the pref to an empty string (in order to make us follow the OS locale) may be a better solution since we have a large number of langpacks installed in that system?

Comment 8

[Mike Kaply [:mkaply]]

> In particular, I'm wondering if setting the pref to an empty string (in order to make us follow the OS locale) may be a better solution since we have a large number of langpacks installed in that system?

There must be some reason they chose not to use matchOS. Unfortunately I wasn't around at the time, so I can't speak to that decision.

Comment 9

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

fair enough.

I'd like to suggest talking to them about testing beta release of a product before they deploy it to their customers. It would give us a chance to work through this issue on much better terms than chemspills.

As I said when we made the change we migrated all the known paths in our systems and this belongs to "dark matter" scope of setups that can only be discovered when the third-party starts being affected. If they don't test beta, it'll hit us on release :(

Comment 10

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

If we can target a system add-on to just their distribution ID, that might be a quicker way to get a fix to affected users.

Comment 11

[Mike Kaply [:mkaply]]

> If we can target a system add-on to just their distribution ID, that might be a quicker way to get a fix to affected users.

I'd be more than happy to try to make that happen. How do I even kick that off?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74826e993054
Special case general.useragent.locale in distributions. r=gandalf

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/74826e993054

Comment 14

[Mike Kaply [:mkaply]]

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

Approval Request Comment
[Feature/Bug causing the regression]: Switch of general.useragent.locale
[User impact if declined]: Locales don't work for acer
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Non
[Is the change risky?]: No
[Why is the change risky/not risky?]: Distribution only
[String changes made/needed]: None

I'd like to preemptively put this on the release in case we do a dot release.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

Let's get this into 60.0b9 anyway.

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5f681bc09309

Comment 17

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

Let's get this onto m-r in case we have another 59 release.

Comment 18

[Andrei Vaida [:avaida]]

Release QA would be interested to work on this sooner than later, if manual testing can be of value here. Mike, could you please help us define a test plan?

Comment 19

[Mike Kaply [:mkaply]]

Sure. To test this, you need to have a language pack installed into Firefox other than the default.

Create a directory called distribution in the same directory as the Firefox executable on Windows.

Place the contents of this file in that directory.

[Global]
id=acer-002
version=2.0
about=Mozilla Firefox for Acer

[XRE]
EnableProfileMigrator=false

[Preferences]
app.distributor="acer"
app.distributor.channel=""
mozilla.partner.id="acer"
browser.shell.checkDefaultBrowser=false
browser.usedOnWindows10=true
extensions.partnerdefaults.checkDefaultDelay=0
extensions.partnerdefaults.checkDefaultDays=2
extensions.partnerdefaults.firstRunDone=false
extensions.partnerdefaults.showBookmarksToolbar=false

[LocalizablePreferences]
browser.search.defaultenginename="Bing"
browser.search.order.1="Bing"
browser.search.order.2="Yahoo"
browser.search.order.3="Google"
browser.startup.homepage="http://acer17win10.msn.com/?pc=ACTE"
startup.homepage_welcome_url="about:blank"
startup.homepage_welcome_url.additional="about:blank"
browser.usedOnWindows10.introURL="about:blank"

[Preferences]
general.useragent.locale="zh-CN"
browser.tabs.drawInTitlebar=false



Modify the line

general.useragent.locale="zh-CN"

To be the non default locale you installed.

Verify that when you start the browser, the non default locale is now the default.

You can also check in about:config to verify that

intl.locale.requested 
is set
NOT general.useragent.locale

Comment 20

[Andrei Vaida [:avaida]]

Thanks a lot for taking the time to write down this information, Mike! We'll start testing it as soon as possible.

Comment 21

[Oana Botisan, Desktop QA]

I managed to reproduce the bug using Firefox 59.0.2 on Windows 10 x64. After I did all the steps from comment 19, the language didn't change, the pref intl.locale.requested wasn't set to ro (I installed the ro language pack), only general.useragent.locale was set. 

I retested everything using the same methods on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13. The builds I used were beta 60.0b9 and latest Nightly. The bug is not reproducing anymore. In these builds the language changes (you have to wait a few seconds ~30s wihtout restarting the browser) and the pref intl.locale.requested is changed to ro and not the other one.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8963971 [details]
Bug 1450270 - Special case general.useragent.locale in distributions.

m-r is on Fx60 now which we're shipping next week