Closed Bug 1252627 Opened 5 years ago Closed 5 years ago
.searchplugins .default Locale should take precedence over general .useragent .locale when looking for engines
If you put an en-US directory in searchplugins/locales, it causes the distribution.searchplugins.defaultLocale pref to be ignored on an en-US build. 1. This should work. 2. We should be honoring distribution.searchplugins.defaultLocale above general.useragent.locale. You should be able to ship an en-US build with an en-HK search engine and use distribution.searchplugins.defaultLocale to make it the default.
The code only checks for the existence of the directory, not that it has files. We can't do much about that. But I do firmly believe we should move the distribution locale code above the useragent locale.
Summary: The presence of an en-US searchplugins/locale directory in distribution dir causes distribution.searchplugins.defaultLocale to be ignored → distribution.searchplugins.defaultLocale should take precedence over general.useragent.locale when looking for engines
This is the desktop patch. We'll need a similar patch for android. This patch only affects affects distributions - there is no other reason someone would set distribution.searchplugins.defaultLocale. Going forward, this allows us to properly have all locale specific engines in one build.
The other diff had extra stuff in it.
Comment on attachment 8726234 [details] [diff] [review] Smaller diff this looks fine to me, are there tests that cover this code?
Attachment #8726234 - Flags: review?(mixedpuppy) → review+
> this looks fine to me, are there tests that cover this code? Sadly no. There are some distribution tests, but they don't cover this. I'll see what I can come up with.
This adds a test that verifies that even when there is an en-US directory, the default locale preference overrides the installed engine. That's why the test engine is named Google, to verify that. I did a quick check of our distributions and not a single one of them uses the locale directory, so this will have no effect on them (and as a side note, we should fix that). The Mozilla Chinese distribution uses the locale directory, but only has the zh-CN directory, so again, no effect. This is mainly needed when we attempt to build larger distributions with multiple engines.
Comment on attachment 8726815 [details] [diff] [review] Diff with test Does the engine file need to be removed in the cleanup function?
(In reply to Shane Caraveo (:mixedpuppy) from comment #7) > Comment on attachment 8726815 [details] [diff] [review] > Diff with test > > Does the engine file need to be removed in the cleanup function? Yes, probably should. Adding.
I'll just remove the whole distribution dir. Any tests that need it create it.
Attachment #8726886 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/integration/fx-team/rev/99e6ed34499483d39ada0c501ffd736116f35828 Bug 1252627 - distribution defaultLocale shoudld take precedence over user agent locale; r=mixedpuppy
https://hg.mozilla.org/integration/fx-team/rev/6f19b1a57959a2e918096c0d66c325ec39583e91 Bug 1252627 - Remove trailing space for eslint to fix bustage - CLOSED TREE
You need to log in before you can comment on or make changes to this bug.