1391213 - RTL control buttons on the wrong side when general.userAgent.locale pref doesn't match locale in use

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Anas El Husseini]

Attachment: Snapshot showing the control buttons on the wrong side

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170814072924

Steps to reproduce:

1- Open Firefox 55 on MS Windows 7.

Note: Firefox was restarted in safe mode and the issue still persisted.


Actual results:

Control buttons (minimize, maximize, close) are showing on the wrong side, thus overlapping with Menu Toolbar or with Tab titles.


Expected results:

Control buttons should be displayed on the other side (left side) in the Right-to-Left version of Firefox.

This seems to be regression, since this issue was not present in Firefox 54.

Comment 1

[:Gijs (he/him)]

Would it be possible for you to use mozregression ( http://mozilla.github.io/mozregression/ ) to work out when this regressed? Because I assume this depends on the language of Windows rather than Firefox, and I don't know anyone in the frontend team with access to RTL versions of Windows...

Comment 2

[:Gijs (he/him)]

Jim, any idea what's going on here? Feels like a widget bug to me, but maybe I'm missing something?

Comment 3

[Jim Mathies [:jimm]]

We set this for composited desktops [1] based on a flag that gets passed in during create through the nsWidgetInitData structure. 

Anas, would you mind posting your about:support text?

[1] http://searchfox.org/mozilla-central/source/widget/windows/nsWindow.cpp#852

Comment 4

[Anas El Husseini]

(In reply to Jim Mathies [:jimm] from comment #3)
> Anas, would you mind posting your about:support text?

about:support output: http://paste.mozilla.org/AnuDXvkR3oOc23ZA

(In reply to :Gijs from comment #1)
> Would it be possible for you to use mozregression (
> http://mozilla.github.io/mozregression/ ) to work out when this regressed?
> Because I assume this depends on the language of Windows rather than
> Firefox, and I don't know anyone in the frontend team with access to RTL
> versions of Windows...

You can install the "Force RTL" addon on an english version of Firefox and use it, and you'll obtain the same visual behavior happening in RTL versions.

I am currently running mozregression and I'll get back to you with the regression window shortly.

Comment 5

[Anas El Husseini]

I finished the mozregression for the range between fx release 52 and 55, and all surprisingly showed the widget buttons on the wrong side. This was illogical, since the issue appeared only recently. So I rechecked and restarted Firefox in safe mode but the issue persisted. Even removing firefox test pilot and its installed experiments didn't change the outcome back to its normal state.

Finally, it occured to me to try Firefox with a fresh clean profile, which I did, and the issue went away then (widget buttons appeared in the right place). I assume that implies that the problem is somewhere in my profile not in firefox code. Any hint on how to pinpoint the cause from here?

And by the way, I forgot to mention it before but my system language is English (US), although I am using the localized version of Firefox.

Comment 6

[:Gijs (he/him)]

(In reply to Anas El Husseini from comment #5)
> I finished the mozregression for the range between fx release 52 and 55, and
> all surprisingly showed the widget buttons on the wrong side. This was
> illogical, since the issue appeared only recently. So I rechecked and
> restarted Firefox in safe mode but the issue persisted. Even removing
> firefox test pilot and its installed experiments didn't change the outcome
> back to its normal state.
> 
> Finally, it occured to me to try Firefox with a fresh clean profile, which I
> did, and the issue went away then (widget buttons appeared in the right
> place). I assume that implies that the problem is somewhere in my profile
> not in firefox code. Any hint on how to pinpoint the cause from here?

The usual method is to disable all your add-ons, restart, see if it's still broken, and assuming it's not, re-enabling add-ons until you see the problem again, to figure out which add-on is causing the problem.

If disabling all your add-ons doesn't help, it's possible the issue might be with one of the custom preferences in your profile, so you could try backing up the prefs file from your profile and replacing it with an empty one, and/or one with only some of the custom preferences. If it's not the add-ons, my first guess would be the layers/gfx preferences that are set in your profile (it especially seems layers acceleration is turned off, but I don't know if that's normal or not, or if it's related...).

Comment 8

[Dani]

After trying many things, I've noticed that the error occurs in the file "prefs.js" in the profile directory.
I will look further into it and try to find the cause to this issue.

Comment 9

[Dani]

(In reply to Anas El Husseini from comment #5)
> I finished the mozregression for the range between fx release 52 and 55, and
> all surprisingly showed the widget buttons on the wrong side. This was
> illogical, since the issue appeared only recently. So I rechecked and
> restarted Firefox in safe mode but the issue persisted. Even removing
> firefox test pilot and its installed experiments didn't change the outcome
> back to its normal state.
> 
> Finally, it occured to me to try Firefox with a fresh clean profile, which I
> did, and the issue went away then (widget buttons appeared in the right
> place). I assume that implies that the problem is somewhere in my profile
> not in firefox code. Any hint on how to pinpoint the cause from here?
> 
> And by the way, I forgot to mention it before but my system language is
> English (US), although I am using the localized version of Firefox.


(In reply to :Gijs from comment #6)
> (In reply to Anas El Husseini from comment #5)
> > I finished the mozregression for the range between fx release 52 and 55, and
> > all surprisingly showed the widget buttons on the wrong side. This was
> > illogical, since the issue appeared only recently. So I rechecked and
> > restarted Firefox in safe mode but the issue persisted. Even removing
> > firefox test pilot and its installed experiments didn't change the outcome
> > back to its normal state.
> > 
> > Finally, it occured to me to try Firefox with a fresh clean profile, which I
> > did, and the issue went away then (widget buttons appeared in the right
> > place). I assume that implies that the problem is somewhere in my profile
> > not in firefox code. Any hint on how to pinpoint the cause from here?
> 
> The usual method is to disable all your add-ons, restart, see if it's still
> broken, and assuming it's not, re-enabling add-ons until you see the problem
> again, to figure out which add-on is causing the problem.
> 
> If disabling all your add-ons doesn't help, it's possible the issue might be
> with one of the custom preferences in your profile, so you could try backing
> up the prefs file from your profile and replacing it with an empty one,
> and/or one with only some of the custom preferences. If it's not the
> add-ons, my first guess would be the layers/gfx preferences that are set in
> your profile (it especially seems layers acceleration is turned off, but I
> don't know if that's normal or not, or if it's related...).

I'd like to thank you two for directing me to the issue,
the cause to this problem is in the file "prefs.js" inside the profile directory, after reading the file and trying to delete sections of it I've noticed this line:

> user_pref("general.useragent.locale", "en-US");

After removing this specific line everything was great, the control buttons were on the left side as they should.
for some reason, updating Firefox to 55 added this line to the profile and caused the control buttons to be on the right side of the screen (I guess it makes sense if the locale is en-US in the prefs...)

removing this line solves the issue.
Thanks again!

Comment 10

[Anas El Husseini]

(In reply to xmegasinx500 from comment #9)
> (In reply to Anas El Husseini from comment #5)
> > I finished the mozregression for the range between fx release 52 and 55, and
> > all surprisingly showed the widget buttons on the wrong side. This was
> > illogical, since the issue appeared only recently. So I rechecked and
> > restarted Firefox in safe mode but the issue persisted. Even removing
> > firefox test pilot and its installed experiments didn't change the outcome
> > back to its normal state.
> > 
> > Finally, it occured to me to try Firefox with a fresh clean profile, which I
> > did, and the issue went away then (widget buttons appeared in the right
> > place). I assume that implies that the problem is somewhere in my profile
> > not in firefox code. Any hint on how to pinpoint the cause from here?
> > 
> > And by the way, I forgot to mention it before but my system language is
> > English (US), although I am using the localized version of Firefox.
> 
> 
> (In reply to :Gijs from comment #6)
> > (In reply to Anas El Husseini from comment #5)
> > > I finished the mozregression for the range between fx release 52 and 55, and
> > > all surprisingly showed the widget buttons on the wrong side. This was
> > > illogical, since the issue appeared only recently. So I rechecked and
> > > restarted Firefox in safe mode but the issue persisted. Even removing
> > > firefox test pilot and its installed experiments didn't change the outcome
> > > back to its normal state.
> > > 
> > > Finally, it occured to me to try Firefox with a fresh clean profile, which I
> > > did, and the issue went away then (widget buttons appeared in the right
> > > place). I assume that implies that the problem is somewhere in my profile
> > > not in firefox code. Any hint on how to pinpoint the cause from here?
> > 
> > The usual method is to disable all your add-ons, restart, see if it's still
> > broken, and assuming it's not, re-enabling add-ons until you see the problem
> > again, to figure out which add-on is causing the problem.
> > 
> > If disabling all your add-ons doesn't help, it's possible the issue might be
> > with one of the custom preferences in your profile, so you could try backing
> > up the prefs file from your profile and replacing it with an empty one,
> > and/or one with only some of the custom preferences. If it's not the
> > add-ons, my first guess would be the layers/gfx preferences that are set in
> > your profile (it especially seems layers acceleration is turned off, but I
> > don't know if that's normal or not, or if it's related...).
> 
> I'd like to thank you two for directing me to the issue,
> the cause to this problem is in the file "prefs.js" inside the profile
> directory, after reading the file and trying to delete sections of it I've
> noticed this line:
> 
> > user_pref("general.useragent.locale", "en-US");
> 
> After removing this specific line everything was great, the control buttons
> were on the left side as they should.
> for some reason, updating Firefox to 55 added this line to the profile and
> caused the control buttons to be on the right side of the screen (I guess it
> makes sense if the locale is en-US in the prefs...)
> 
> removing this line solves the issue.
> Thanks again!

Tested and confirmed. After changing "general.useragent.locale" to "ar" in about:config and restarting Firefox, the control buttons went back to the correct place. Thanks for finding that out.

I guess what is needed to fix this issue is at least two things:
- Making sure the default value of general.useragent.locale is set to the browser locale, and not to the OS locale for instance.
- In case a different langpack is installed and later removed/disabled, make sure general.useragent.locale is reset to its original value (I say this because mine was set to 'en-GB' after I tried an en-GB langpack on the Arabic Firefox then removed it).

Thanks.

Comment 11

[:Gijs (he/him)]

(In reply to Anas El Husseini from comment #10)
> - In case a different langpack is installed and later removed/disabled, make
> sure general.useragent.locale is reset to its original value (I say this
> because mine was set to 'en-GB' after I tried an en-GB langpack on the
> Arabic Firefox then removed it).

Gandalf, can you look into why the locale service apparently gives the wrong answer some of the time, or something?

Comment 12

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

> Gandalf, can you look into why the locale service apparently gives the wrong answer some of the time, or something?

Can I get STR for getting LocaleService to give wrong answer?

Is it about langpack being uninstalled and LocaleService remaining with wrong locales until restart? Something else?

If it is the one scenario I described - we have a lot of bugs related to the old langpacks. We're about to switch to webext-based language packs (bug 1395363) which will fix this scenario.

Comment 13

[Anas El Husseini]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
> > Gandalf, can you look into why the locale service apparently gives the wrong answer some of the time, or something?
> 
> Can I get STR for getting LocaleService to give wrong answer?
> 
> Is it about langpack being uninstalled and LocaleService remaining with
> wrong locales until restart? Something else?
> 
> If it is the one scenario I described - we have a lot of bugs related to the
> old langpacks. We're about to switch to webext-based language packs (bug
> 1395363) which will fix this scenario.

I tried to find the steps to reproduce, but there are too many unknowns here. The original scenario that led to this bug is having an old profile on an RTL Firefox 54 (on Windows 7), then having it updated to version 55, and then the bug appeared immediately after the restart, caused by the general.useragent.locale pref.

So I can't say for sure if this is bound to happen again if you reproduce the same scenario above. Maybe some of the installed extensions had some role in this...

As for the langpack, since they are going to be deprecated soon in this format, I don't think it is worth investigating further, especially since we are not sure if they caused this themselves or was affected by the original bug.

Comment 14

[Dani]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
> > Gandalf, can you look into why the locale service apparently gives the wrong answer some of the time, or something?
> 
> Can I get STR for getting LocaleService to give wrong answer?
> 
> Is it about langpack being uninstalled and LocaleService remaining with
> wrong locales until restart? Something else?
> 
> If it is the one scenario I described - we have a lot of bugs related to the
> old langpacks. We're about to switch to webext-based language packs (bug
> 1395363) which will fix this scenario.

I've encountered this bug after updating Firefox 54 with "he-IL" locale (RTL) to Firefox 55.
Never installed any other langpack.

Comment 15

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
> > Gandalf, can you look into why the locale service apparently gives the wrong answer some of the time, or something?
> 
> Can I get STR for getting LocaleService to give wrong answer?

TBH, I'm not sure, but see the other commenters. It seems clear that there are cases where this:

https://dxr.mozilla.org/mozilla-central/rev/fd87bb184e299fec695f69bd2977276c25719b98/xpfe/appshell/nsAppShellService.cpp#734

(which gets passed into nsWindow and used per comment #3)

doesn't end up matching what the rest of the UI does/thinks. My guess is that it's reading the pref (which afaict from the 2 reported cases ends up not matching the locale we actually end up using to display the UI) and then returning the wrong thing.

> Is it about langpack being uninstalled and LocaleService remaining with
> wrong locales until restart? Something else?
> 
> If it is the one scenario I described - we have a lot of bugs related to the
> old langpacks. We're about to switch to webext-based language packs (bug
> 1395363) which will fix this scenario.

If I'm right about the pref mismatch thing then I don't know that that will help here.

I'm really not an expert here, so I likely can't help with STR or the like. I'm just trying to get this bug on the radar of the right people. Right now that looks like it might be you. :-)

Comment 16

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

Sure, thank you for flagging me! I am the right person :)

> My guess is that it's reading the pref (which afaict from the 2 reported cases ends up not matching the locale we actually end up using to display the UI) and then returning the wrong thing.

Except that it doesn't read any pref. In Fx desktop we have ICU for quite a while, so all we do is we take the UI locale and run it via ICU's uloc_isRightToLeft ( http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#509 ).

If people are getting wrong RTL bit it may be that some code in our front end doesn't use LocaleService::IsAppLocaleRTL API and instead tries to read some pref manually and that introduces inconsistency.

I need some STR to debug it. Let's keep it open and if you see anything which may help me isolate where it happens, flag me pls!

Comment 17

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #16)
> Sure, thank you for flagging me! I am the right person :)
> 
> > My guess is that it's reading the pref (which afaict from the 2 reported cases ends up not matching the locale we actually end up using to display the UI) and then returning the wrong thing.
> 
> Except that it doesn't read any pref. In Fx desktop we have ICU for quite a
> while, so all we do is we take the UI locale and run it via ICU's
> uloc_isRightToLeft (
> http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.
> cpp#509 ).

Err, so to get the UI locale, it reads the UI locale from the pref, no? ( http://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/intl/locale/LocaleService.cpp#98-101 )

From a very naive read, it looks like the code also hardcodes a fallback to en-US, which I believe isn't present in the fully-localized builds (as opposed to en-US with a non-en-US langpack).

> If people are getting wrong RTL bit it may be that some code in our front
> end doesn't use LocaleService::IsAppLocaleRTL API and instead tries to read
> some pref manually and that introduces inconsistency.

Well, on Windows 7 we use native title bar buttons. I would expect their placement to be entirely controlled by widget and/or the OS, but I could be wrong.

> I need some STR to debug it. Let's keep it open and if you see anything
> which may help me isolate where it happens, flag me pls!

I can reproduce using the following steps:

0. on win7
1. download 54.0.1 arabic ( http://archive.mozilla.org/pub/firefox/releases/54.0.1/win64/ar/ (or win32, depending on your copy of win7)
2. install
3. create a new profile
4. open. Note that the titlebar button appears in the correct place (left-hand-side of the window).
5. in about:config, toggle general.useragent.locale to 'en-US'
6. restart (you may need to rm firefox and reinstall with the saved complete installer if firefox has finished downloading the update by now to verify). Note that the titlebar buttons still appears in the correct place.
7. update to 55.
8. titlebar buttons appear in the wrong place.

Comment 18

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

:Gijs, I tried to reproduce the problem.

In the code, I agree that we shouldn't fallback on en-US if the only locale data we have is `ar` (and we do right now).

So i see in the console:

Services.locale.getRequestedLocales{); // en-US
Services.locale.getAvailableLocales(); // ar
Services.locale.getAppLocalesAsLangTags(); // ['en-US']
Services.locale.isAppLocaleRTL; // false;

I cannot reproduce it visually in Windows 10, but I guess that we use the isAppLocaleRTL in that scenario and it returns the wrong value.

I think the solution is to use AppConstants.INSTALL_LOCALE as the default instead en-US.

Comment 19

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

Jonathan, how can I get access to the equivalent of AB_CD or AppConstants.INSTALL_LOCALE or the content of `update.locale` file in omni.ja from C++?

I think we need to set the LocaleService.defaultLocale to match it instead of assuming that it's always en-US.

Comment 20

[Jonathan Kew [:jfkthame]]

Attachment: Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value

IIUC, we can't do anything that involves a value compiled-in to the C++ code (even if passed by #define from the build system, or whatever) because it may change post-compilation in a repack step. So the C++ code is going to have to retrieve it from somewhere. We could put it into a pref string (which could then be adjusted during repacks), which would be trivial to read via Preferences API, but that invites the possibility of the pref getting out of sync with the actual package. So perhaps reading update.locale is the simplest and most robust option. We should only need to read it once and then cache it, as it won't change at runtime. So I think something like this (only minimally tested) ought to work, AFAICS.

Comment 21

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

Are you sure that this code will work always? I'm not very familiar with our packaging but I thought we sometimes work without omni.ja code.

It also feels pretty heavy handed for a single string that is known at build time as @AB_CD@ :( Is there any way to generate it during build time and somehow place into an .h or .inc file that would be inlined into the C++ file?

Comment 22

[Jonathan Kew [:jfkthame]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #21)
> Are you sure that this code will work always? I'm not very familiar with our
> packaging but I thought we sometimes work without omni.ja code.

That's true for a developer running a local build without the packaging step. The patch I wrote would just default to a hard-coded en-US in this case.

> 
> It also feels pretty heavy handed for a single string that is known at build
> time as @AB_CD@ :(

That's the question... is it known at C++ compile time, or only at packaging time? I thought it's the latter but I don't really know much about our process.

> Is there any way to generate it during build time and
> somehow place into an .h or .inc file that would be inlined into the C++
> file?

That depends. I was under the impression that it could be changed *after* "build" time (compilation) during localization (isn't that what a "repack" does?). If that's true, then we can't make it a compile-time constant in C++, because then it'll be frozen in the compiled code and the repack won't affect it.

Comment 23

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

Comment on attachment 8908625 [details] [diff] [review]
Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value

Good point. Let's land it to fix this bug and we can fine tune later.

For developer builds that are not packaged I believe we always have en-US coming from build, even if we then repackaged with another locale, so fallback on en-US makes sense.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11bbf95a3db6
Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value. r=emk

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage, at least on Linux:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef5d81c6af3b8b75608b822aee83edc57b1f3822

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=11bbf95a3db6099861f50c807b251caf392f3d14&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=132827526&repo=mozilla-inbound

[task 2017-09-23T09:30:48.539Z] 09:30:48     INFO - =================================== FAILURES ===================================
[task 2017-09-23T09:30:48.539Z] 09:30:48     INFO - ______________________ XPCShellTestsTests.testAddTaskSkip ______________________
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO - self = <selftest.XPCShellTestsTests testMethod=testAddTaskSkip>
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO -     def testAddTaskSkip(self):
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO -         self.writeFile("test_tasks_skip.js", ADD_TASK_SKIP)
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO -         self.writeManifest(["test_tasks_skip.js"])
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO - >       self.assertTestResult(True)
[task 2017-09-23T09:30:48.540Z] 09:30:48     INFO - ../testing/xpcshell/selftest.py:1068:
[task 2017-09-23T09:30:48.541Z] 09:30:48     INFO - _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[task 2017-09-23T09:30:48.541Z] 09:30:48     INFO - ../testing/xpcshell/selftest.py:527: in assertTestResult
[task 2017-09-23T09:30:48.541Z] 09:30:48     INFO -     """ % ("passed" if expected else "failed", self.log.getvalue()))
[task 2017-09-23T09:30:48.541Z] 09:30:48     INFO - E   AssertionError: Tests should have passed, log:
[task 2017-09-23T09:30:48.541Z] 09:30:48     INFO - E   ========
[task 2017-09-23T09:30:48.542Z] 09:30:48     INFO - E   MOZ_NODE_PATH environment variable not set. Tests requiring http/2 will fail.
[task 2017-09-23T09:30:48.542Z] 09:30:48     INFO - E   Running tests sequentially.
[task 2017-09-23T09:30:48.542Z] 09:30:48     INFO - E   SUITE-START | Running 1 tests
[task 2017-09-23T09:30:48.542Z] 09:30:48     INFO - E   TEST-START | test_tasks_skip.js
[task 2017-09-23T09:30:48.542Z] 09:30:48  WARNING - E   TEST-UNEXPECTED-FAIL | test_tasks_skip.js | xpcshell return code: -11
[task 2017-09-23T09:30:48.543Z] 09:30:48     INFO - E   TEST-INFO took 142ms
[task 2017-09-23T09:30:48.543Z] 09:30:48     INFO - E   >>>>>>>
[task 2017-09-23T09:30:48.543Z] 09:30:48     INFO - E   PID 36303 | Hit MOZ_CRASH(ElementAt(aIndex = 0, aLength = 0)) at /builds/worker/workspace/build/src/xpcom/ds/nsTArray.cpp:28
[task 2017-09-23T09:30:48.543Z] 09:30:48     INFO - E   PID 36303 | ExceptionHandler::GenerateDump cloned child 36312
[task 2017-09-23T09:30:48.544Z] 09:30:48     INFO - E   PID 36303 | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2017-09-23T09:30:48.544Z] 09:30:48     INFO - E   PID 36303 | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2017-09-23T09:30:48.544Z] 09:30:48     INFO - E   <<<<<<<

Comment 26

[Jonathan Kew [:jfkthame]]

This really puzzled me at first, as the log doesn't appear to show anything remotely connected to the patch that was landed. But I think it's actually a simple bug in the GetDefaultLocale patch; in the non-packaged fallback case, it assigns the hardcoded en-US to aRetVal, but then promptly overwrites it with the (empty) mDefaultLocale value. :( It should be assigning the hardcoded fallback to that member instead, so that it sticks around.

I'll re-land after tryserver confirms the fix.

Comment 27

[Jonathan Kew [:jfkthame]]

Looks happier: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb7b393f7b5893efe373f3e309cae8a2d63b4b12

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/08ff4ead034b
Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value. r=emk

Comment 29

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/08ff4ead034b

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 31

[Jonathan Kew [:jfkthame]]

Before we request uplift here, I'd appreciate someone verifying with latest Nightly that it has in fact fixed the issue that was reported.

(Leaving needinfo? for now, adding qawanted.)

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Brindusa, can you help with this?

Comment 33

[roxleitan]

Tested on Windows 7 x64 and Windows 7 x86 using the STR from comment 17 and didn't manage to reproduce this bug. 

I also tried with 54.0.1 "he" build with a new profile(p1) and "no update" option and "general.useragent.locale" pref modified to 'en-US, then installed 55.0.2 (Build ID: 20170814072924) and opened with the same p1 profile and couldn't reproduce it.

Anas El Husseini, Dani, could you please check using your profiles(the ones that you used when encountered the bugs) using the latest Nightly 58.0a1 if the issue is still reproducible?

Comment 34

[Anas El Husseini]

Tested on the latesst Arabic nightly build with my old profile. Issue is gone.

Comment 35

[Jonathan Kew [:jfkthame]]

Comment on attachment 8908625 [details] [diff] [review]
Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value

Approval Request Comment
[Feature/Bug causing the regression]: refactoring of i18n/locale services
[User impact if declined]: UI glitches (window controls on wrong side) in some cases, depending on product and system locales
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: yes, by QA and by reporter (see above)
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: change is limited to just the GetDefaultLocale method, and if the new code fails it falls back to the existing hard-coded default
[String changes made/needed]: none

Comment 36

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8908625 [details] [diff] [review]
Make localeService.defaultLocale return the locale of the app package rather than a hard-coded en-US value

This doesn't seem like a recent regression. Sorry cannot take it in 57. Recommend letting this ride the 58 train.