1057631 - Use real locale when pulling search engine out of the jar

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect, P1)

Description

[:Margaret Leibovic]

We'll want to fix bug 1057629 before we do this.

Comment 1

[:Margaret Leibovic]

Background: we're hacking together a path to pull search plugins out of the jar, and right now we hard-code it to use the en-US path:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java#160

I'm not sure of the best way to go about fixing this bug. My first instinct is to just use the system locale, but I realized that if we're using strings from Fennec for the search activity (bug 1024527), the search activity may be in a gecko locale that's different from the system's.

rnewman, where do we store the current locale for Fennec? Is there a SharedPreference I can read?

Comment 2

[Richard Newman [:rnewman]]

So long as all of our application entry points do the right BrowserLocaleManager dance, you can just use Locale.getDefault(): in the course of applying the user's preference, we set Locale.

(You should be doing that BrowserLocaleManager dance in the search activity, too. See <https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/fennec/localeswitching.html>.)

getAndApplyPersistedLocale -- which those docs suggest you should use -- actually returns the locale code for you, so you can hold onto it when you leave the dance floor and start looking for a search plugin.

Comment 3

[:Margaret Leibovic]

Attachment: WIP - Use real locale when pulling search engine out of the jar

First of all, I'm not sure about this string replace to swap the underscore for the dash. Is there any place in the tree where we already have to handle something like this?

Secondly, I'm not sure about the fallback to look for just the language. How do we currently handle the case where the system has a region-specific locale, but we only support a language (e.g. fr_FR and fr).

Comment 4

[Richard Newman [:rnewman]]

(In reply to :Margaret Leibovic from comment #3)

> First of all, I'm not sure about this string replace to swap the underscore
> for the dash. Is there any place in the tree where we already have to handle
> something like this?

BrowserLocaleManager.getLanguageTag will do the right thing for a given Locale object.


> Secondly, I'm not sure about the fallback to look for just the language. How
> do we currently handle the case where the system has a region-specific
> locale, but we only support a language (e.g. fr_FR and fr).

Various ad hoc methods. E.g., see chrome/nsChromeRegistryChrome.cpp:54, which does; versus browser/components/dirprovider/DirectoryProvider.cpp, which doesn't fall back.

Desktop doesn't really have this problem: you install a Firefox in a language you picked, and you can't change it. On Android we're in a slightly different situation, because the OS locale might be a sibling or child of a supported locale (es-US => es-ES, fr-CA => fr). A fallback is a good idea in that case.

(For strings we fall back, but we don't fall sideways: Bug 933315.)

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8483830 [details] [diff] [review]
WIP - Use real locale when pulling search engine out of the jar

Review of attachment 8483830 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java
@@ +219,5 @@
> +    private InputStream getInputStreamFromJar(String fileName) {
> +        final Locale locale = Locale.getDefault();
> +
> +        // First, try a file path for the full locale.
> +        String url = getSearchPluginsJarUrl(locale.toString().replace("_", "-"), fileName);

You can use BLM.getLanguageTag(locale) here.

@@ +227,5 @@
> +            return in;
> +        }
> +
> +        // If that doesn't work, try a file path for just the language.
> +        url = getSearchPluginsJarUrl(locale.getLanguage(), fileName);

You probably want to check if localeTag.equals(language), and don't try twice for `fr` and friends.

Bear in mind that Java gets language tags wrong; see the translation code in getLanguageTag.

I encourage you to add a getLanguage(locale) method to BLM, which would do only the first part of getLanguageTag(locale).

@@ +234,5 @@
>  
>      /**
>       * Gets the jar URL for a file in the searchplugins directory
>       *
> +     * @param locale String representing the gecko locale (e.g. "en-US")

Nit: s/gecko/Gecko, throughout.

@@ +241,2 @@
>       */
> +    private String getSearchPluginsJarUrl(String locale, String fileName) {

Nit: s/Url/URL, throughout.

Comment 6

[:Margaret Leibovic]

Attachment: Use real locale when pulling search engine out of the jar

This appears to be working.

Corresponding PR: https://github.com/mozilla/fennec-search/pull/80

I just had to add a stub implementation for getLanguageTag. It doesn't really matter what it does, since the standalone activity will never successfully get anything out of the jar directory (that's why we have a fallback to read search plugins from assets).

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8484573 [details] [diff] [review]
Use real locale when pulling search engine out of the jar

Review of attachment 8484573 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with this change.

::: mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java
@@ +229,5 @@
> +            return in;
> +        }
> +
> +        // If that doesn't work, try a file path for just the language.
> +        final String language = locale.getLanguage();

= BrowserLocaleManager.getLanguage(locale);

Comment 8

[:Margaret Leibovic]

https://github.com/mozilla/fennec-search/commit/b9e05c06492ba7f36e74ad39edf84a1672953e55
https://hg.mozilla.org/integration/fx-team/rev/f33e9c8cd993

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f33e9c8cd993