1434805 - Switch to use LocaleList on Android once we start using API Level 24

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

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

At the moment we only retrieve a single "OS locale" from Android, but starting with API level 24 we can use LocaleList[0] to get a list of locales.

That would be useful for us to help us select the best available locale for the user, complete the OSPreferences hooks and also improve our glyph selection.


[0] https://developer.android.com/reference/android/os/LocaleList.html#getDefault

Comment 1

[Nick Alexander :nalexander [he/him]]

There's no need to wait to build with API 24.  We can check at runtime for API >= 24 and then dynamically invoke the new method using reflection.  There are many such examples in the tree, e.g., https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/WindowUtils.java#15.

There is almost certainly more work involved in adapting to lists than in exposing LocaleList.getDefault() in some way.

Comment 2

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=32bc63a4b515faa6dffc8074402fc16f5ecf6d82

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Bug 1434805 - Use LocaleList on Android N+.

Android N+ has new Locale mode that selects multiple locale for font selection.
We should support it to be better locale system selection.

And, Locale.toLanguageTag will return zh-Hant-TW instead of zh-TW for Taiwan
locale on Android O+, so we shouldn't use Locale.toLanguageTag for current
Gecko locale support.  (Android M will return zh-TW)

Review commit: https://reviewboard.mozilla.org/r/250520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250520/

Comment 4

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

> And, Locale.toLanguageTag will return zh-Hant-TW instead of zh-TW for Taiwan
locale on Android O+, so we shouldn't use Locale.toLanguageTag for current
Gecko locale support.

Why do you say that? What will not work? Gecko's LocaleService should be able to parse `zh-Hant-TW` and match it against `zh-TW` language pack, and I believe at this point we handle everything via language negotiation so it should work. Am I missing something?

Comment 5

[Makoto Kato [:m_kato]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> > And, Locale.toLanguageTag will return zh-Hant-TW instead of zh-TW for Taiwan
> locale on Android O+, so we shouldn't use Locale.toLanguageTag for current
> Gecko locale support.
> 
> Why do you say that? What will not work? Gecko's LocaleService should be
> able to parse `zh-Hant-TW` and match it against `zh-TW` language pack, and I
> believe at this point we handle everything via language negotiation so it
> should work. Am I missing something?

Ah, I understand.  I may use toLanguageTag, but we still need to use org.mozilla.gecko.Locales.toLanguageTag convert some languages to Gecko's for Fennec.

Comment 6

[Makoto Kato [:m_kato]]

Comment on attachment 8984629 [details]
Bug 1434805 - Use LocaleList on Android N+.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250520/diff/1-2/

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8984629 [details]
Bug 1434805 - Use LocaleList on Android N+.

https://reviewboard.mozilla.org/r/250520/#review256942

::: mobile/android/base/java/org/mozilla/gecko/BrowserLocaleManager.java:458
(Diff revision 2)
> +            ArrayList<String> locales = new ArrayList<String>();
>              LocaleManager localeManager = Locales.getLocaleManager();
>              Context context = GeckoAppShell.getApplicationContext();
>              if (!localeManager.isMirroringSystemLocale(context)) {
>                  // User uses specific browser locale instead of system locale
> -                return Locales.getLanguageTag(localeManager.getCurrentLocale(context));
> +                locales.add(Locales.getLanguageTag(localeManager.getCurrentLocale(context)));

Need to return from this if block.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java:1899
(Diff revision 2)
>              return DEFAULT;
>          }
>          return Integer.parseInt(prop);
>      }
>  
> -    @WrapForJNI
> +    private static String getLanguageTag(final Locale aLocale) {

No 'a' prefix for parameters in Java.

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8984629 [details]
Bug 1434805 - Use LocaleList on Android N+.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250520/diff/2-3/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad55ae086eb4
Use LocaleList on Android N+. r=jchen

Comment 10

[Andrei Ciure[:aciure]]

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