In bug 1410736 we migrated the remaining uses of general.useragent.locale in anticipation for bug 1414390. :florian pointed out that we introduced a regression in https://hg.mozilla.org/mozilla-central/rev/f401d9f8a87d#l10.48 Let's fix it.

Comment on attachment 8931461 [details] Bug 1420255 - Fix a regression from bug 1410736. https://reviewboard.mozilla.org/r/202608/#review207924 The change looks good to me. I'm a bit worried this isn't covered by a test, but bug 1289240 comment 3 explains why this is difficult to test. ::: toolkit/components/search/nsSearchService.js:2273 (Diff revision 1) > > // If we are using a non-default locale or in the xpcshell test case, > // we'll accept as a 'default' engine anything that has been registered at > // resource://search-plugins/ even if the file doesn't come from the > // application folder. If not, skip costly additional checks. > - if (Services.locale.defaultLocale !== Services.locale.getRequestedLocale() && > + if (Services.locale.defaultLocale === Services.locale.getRequestedLocale() && nit: Our front-end code usually uses == rather than === for comparisons, unless there's a specific reason to need ===.

[Tracking Requested - why for this release]: I haven't verified this, but I'm afraid this may cause the search reset code in bug 1419941 to be triggered for engines shipped by language packs (which we want to treat as default engines, and not affect with the reset). I'm glad 57 isn't affected.

Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c2832d8c0798 Fix a regression from bug 1410736. r=florian

Comment on attachment 8931461 [details] Bug 1420255 - Fix a regression from bug 1410736. Approval Request Comment [Feature/Bug causing the regression]: bug 1410736 [User impact if declined]: the non-default locale default search engine selection is broken. [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]: none [Is the change risky?]: no [Why is the change risky/not risky?]: it's a simple regression with a single character fix that the author and reviewer of the patch understand. [String changes made/needed]: none

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #6) > [Needs manual test from QE? If yes, steps to reproduce]: no Given this bug has a significant impact on search reset, I think we should verify it please. Steps to reproduce: 1. Install a beta build. 2. Install a langpack. Some are available at https://tools.taskcluster.net/index/artifacts/releases.v1.mozilla-beta.latest.firefox.latest.l10n_artifacts.macosx64.4 3. In about:config, set general.useragent.locale to the locale of the langpack you just installed. 4. Restart the build to have Firefox started in the locale of the langpack. 5. From the browser console (you need to enable it from the devtools settings), verify the following: Services.search.originalDefaultEngine.wrappedJSObject._isDefault This should always be true. Without the fix, it's false. Services.search.getDefaultEngines().map(e => e.name) This should list the names of the available search engines for the current locale. Without the fix this gives an empty list.

serious issue; marking it as blocking the release

Comment on attachment 8931461 [details] Bug 1420255 - Fix a regression from bug 1410736. Fix a 58 blocker. Beta58+.

I verified this issue using Latest Nightly 59.0a1 and Latest Firefox Beta 58.0b7 with Build ID 20171127220446 on Mac OS X 10.12.6 and Mac OS X 10.12. I will mark this as verified fixed.