1252627 - distribution.searchplugins.defaultLocale should take precedence over general.useragent.locale when looking for engines

Status: RESOLVED FIXED

(Firefox :: Distributions, defect)

Description

[Mike Kaply [:mkaply]]

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.

Comment 1

[Mike Kaply [:mkaply]]

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.

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Reverse order of engine seaarch

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.

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Smaller diff

The other diff had extra stuff in it.

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8726234 [details] [diff] [review]
Smaller diff

this looks fine to me, are there tests that cover this code?

Comment 5

[Mike Kaply [:mkaply]]

> 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.

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Diff with test

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 7

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8726815 [details] [diff] [review]
Diff with test

Does the engine file need to be removed in the cleanup function?

Comment 8

[Mike Kaply [:mkaply]]

(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.

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Cleanup after test

I'll just remove the whole distribution dir.

Any tests that need it create it.

Comment 10

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/fx-team/rev/99e6ed34499483d39ada0c501ffd736116f35828
Bug 1252627 - distribution defaultLocale shoudld take precedence over user agent locale; r=mixedpuppy

Comment 11

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/fx-team/rev/6f19b1a57959a2e918096c0d66c325ec39583e91
Bug 1252627 - Remove trailing space for eslint to fix bustage - CLOSED TREE

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/99e6ed344994
https://hg.mozilla.org/mozilla-central/rev/6f19b1a57959