Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2481293004:
Use LocaleList in Android/Webview locale representation (Closed)
|
Created:
4 years, 1 month ago by Yirui Huang Modified:
4 years, 1 month ago CC:
chromium-reviews, android-webview-reviews_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse LocaleList in Android/Webview locale representation
From Android N, uses are able to select multiple languages in system
Settings. This CL provides supports for multi-locales features for
Chrome and System WebView. This does not breaking behaviours on
Andriod M and before.
Since toLanguageTags in LocaleList does not convert Android language
"tl"(Tagalog) to Chromium language "fil"(Filipino), an update for
toLanguageTags is added.
BUG=593515
Committed: https://crrev.com/9112d68d3d59f500ecd45e128337576aedaf5e64
Cr-Commit-Position: refs/heads/master@{#431128}
Patch Set 1 #
Total comments: 28
Patch Set 2 : Revert ResourceExtractor #
Total comments: 12
Patch Set 3 : Add more test cases with minSDKLevel requirement #
Total comments: 2
Patch Set 4 : description changes #
Messages
Total messages: 34 (18 generated)
Description was changed from ========== Enable LocaleList in Android/Webview Add LocaleList without break current behaviours with Locale. Add more tests for a better coverage of API levels. Modified related code in Android Webview and Resource Extractor. BUG=593515 ========== to ========== Add LocaleList in Android/Webview locale representation Add LocaleList related functions without break current behaviors with Locale and more tests for a better coverage of API levels. Relevant code is modified in Android Webview and Resource Extractor. Regarding to toLanguageTags and forLanguageTags, toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino). Whereas forLanguageTags in LocaleList, it converts languages correspondingly. Thus, an update for toLanguageTags is added and LocaleList.forLanguageTags is used for Android N and above. BUG=593515 ==========
yirui@google.com changed reviewers: + nona@chromium.org
yirui@google.com changed reviewers: + ksk@google.com
https://codereview.chromium.org/2481293004/diff/1/android_webview/java/src/or... File android_webview/java/src/org/chromium/android_webview/AwContents.java (right): https://codereview.chromium.org/2481293004/diff/1/android_webview/java/src/or... android_webview/java/src/org/chromium/android_webview/AwContents.java:1009: // Deal with a string represents either single locale or a locale list Then, lets change the argument name to "locales". https://codereview.chromium.org/2481293004/diff/1/base/BUILD.gn File base/BUILD.gn (left): https://codereview.chromium.org/2481293004/diff/1/base/BUILD.gn#oldcode2418 base/BUILD.gn:2418: "android/junit/src/org/chromium/base/LocaleUtilsTest.java", Have you investigated why LocaleList is not available in junit tests? You don't need to look into deeper but good to leave TODO and file a bug for that. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:199: return commandLine.hasSwitch(BaseSwitches.DEFAULT_COUNTRY_CODE_AT_INSTALL) How about checking the commandline before resolving the locale object? https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/ResourceExtractor.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:186: LocaleList localeList = LocaleList.getDefault(); I think we should use LocaleList.getAdjustedDefault(). and not query all languages here. The "adjusted" default means that even if the first language is always capable for displaying contents. BTW, can we always use the first language in adjusted locale list here? https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:127: localeList = new LocaleList(locale1); Looks like this test doesn't work on M and before.
https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:166: Locale locale; How about: for (Locale locale : localeList) { Locale updatedLocale = getUpdatedLocaleForChromium(locale) newLocaleList.add(updatedLocale); } We should minimize lifetime of variables. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:169: newLocaleList.add(toLanguageTag(locale)); Now locale.toLanguageTag is not used. Is it intentional? If so, please update the method comment. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/ResourceExtractor.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:179: Locale defaultLocale; Please declare defaultLocale and localeLanguage in the if or else block, https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:181: if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { ... } else { ... } for consistency. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:187: for (int i = 0; i < localeList.size(); i++) { for (Locale defaultLocale : localeList) { } https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:195: // Currenty (Oct 2016), this array can be as big as 4 entries, so using a capacity Is this comment still correct?
https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:125: * This works for API is lower than 24. For API is 24 and above, LocaleList.toLanguageTags s/LocaleList.toLanguageTags/LocaleList.forLanguageTags/? https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:166: Locale locale; On 2016/11/08 09:38:02, ksk1 wrote: > How about: > for (Locale locale : localeList) { > Locale updatedLocale = getUpdatedLocaleForChromium(locale) > newLocaleList.add(updatedLocale); > } > We should minimize lifetime of variables. for (int i = 0; i < localeList.size(); i++) is better. Please forget about for (Locale locale : localeList)
After reverting the file ResourceExtractor.java, Chromium and Android Webview on Android N still work with multiple accept languages. https://codereview.chromium.org/2481293004/diff/1/android_webview/java/src/or... File android_webview/java/src/org/chromium/android_webview/AwContents.java (right): https://codereview.chromium.org/2481293004/diff/1/android_webview/java/src/or... android_webview/java/src/org/chromium/android_webview/AwContents.java:1009: // Deal with a string represents either single locale or a locale list On 2016/11/08 09:31:11, Seigo Nonaka wrote: > Then, lets change the argument name to "locales". Done. https://codereview.chromium.org/2481293004/diff/1/base/BUILD.gn File base/BUILD.gn (left): https://codereview.chromium.org/2481293004/diff/1/base/BUILD.gn#oldcode2418 base/BUILD.gn:2418: "android/junit/src/org/chromium/base/LocaleUtilsTest.java", On 2016/11/08 09:31:11, Seigo Nonaka wrote: > Have you investigated why LocaleList is not available in junit tests? > You don't need to look into deeper but good to leave TODO and file a bug for > that. Done. TODO statement inserted. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:125: * This works for API is lower than 24. For API is 24 and above, LocaleList.toLanguageTags On 2016/11/08 09:47:13, ksk1 wrote: > s/LocaleList.toLanguageTags/LocaleList.forLanguageTags/? Done. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:166: Locale locale; On 2016/11/08 09:38:02, ksk1 wrote: > How about: > for (Locale locale : localeList) { > Locale updatedLocale = getUpdatedLocaleForChromium(locale) > newLocaleList.add(updatedLocale); > } > We should minimize lifetime of variables. Done. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:166: Locale locale; On 2016/11/08 09:38:02, ksk1 wrote: > How about: > for (Locale locale : localeList) { > Locale updatedLocale = getUpdatedLocaleForChromium(locale) > newLocaleList.add(updatedLocale); > } > We should minimize lifetime of variables. Done. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:169: newLocaleList.add(toLanguageTag(locale)); On 2016/11/08 09:38:02, ksk1 wrote: > Now locale.toLanguageTag is not used. Is it intentional? If so, please update > the method comment. Done. The comment below is the explanation for not using locale.toLanguageTag, I moved it to right position now. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/LocaleUtils.java:199: return commandLine.hasSwitch(BaseSwitches.DEFAULT_COUNTRY_CODE_AT_INSTALL) On 2016/11/08 09:31:11, Seigo Nonaka wrote: > How about checking the commandline before resolving the locale object? Done. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... File base/android/java/src/org/chromium/base/ResourceExtractor.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:179: Locale defaultLocale; On 2016/11/08 09:38:02, ksk1 wrote: > Please declare defaultLocale and localeLanguage in the if or else block, File reverted to the previous version. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:181: if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { On 2016/11/08 09:38:02, ksk1 wrote: > if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { > ... > } else { > ... > } > for consistency. File reverted to the previous version. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:186: LocaleList localeList = LocaleList.getDefault(); On 2016/11/08 09:31:11, Seigo Nonaka wrote: > I think we should use LocaleList.getAdjustedDefault(). and not query all > languages here. > The "adjusted" default means that even if the first language is always capable > for displaying contents. > BTW, can we always use the first language in adjusted locale list here? File reverted to the previous version. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:187: for (int i = 0; i < localeList.size(); i++) { On 2016/11/08 09:38:02, ksk1 wrote: > for (Locale defaultLocale : localeList) { > } File reverted to the previous version. https://codereview.chromium.org/2481293004/diff/1/base/android/java/src/org/c... base/android/java/src/org/chromium/base/ResourceExtractor.java:195: // Currenty (Oct 2016), this array can be as big as 4 entries, so using a capacity On 2016/11/08 09:38:02, ksk1 wrote: > Is this comment still correct? File reverted to the previous version. https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:127: localeList = new LocaleList(locale1); On 2016/11/08 09:31:11, Seigo Nonaka wrote: > Looks like this test doesn't work on M and before. Yes, this is only for Android N. For Android M and before, toLanguageTag is tests in testToLanguageTag.
https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:127: localeList = new LocaleList(locale1); On 2016/11/08 10:39:31, yirui wrote: > On 2016/11/08 09:31:11, Seigo Nonaka wrote: > > Looks like this test doesn't work on M and before. > > Yes, this is only for Android N. For Android M and before, toLanguageTag is > tests in testToLanguageTag. I meant this test also runs on Android M and before, so you need to check API version or you need to put MinAndroidSdkLevel annotation. e.g. https://cs.chromium.org/chromium/src/content/public/android/javatests/src/org... https://codereview.chromium.org/2481293004/diff/20001/android_webview/java/sr... File android_webview/java/src/org/chromium/android_webview/AwContents.java (right): https://codereview.chromium.org/2481293004/diff/20001/android_webview/java/sr... android_webview/java/src/org/chromium/android_webview/AwContents.java:1013: sCurrentLocale = locales; sCurrentLocales as well? https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:125: * This works for API is lower than 24. For API is 24 and above, LocaleList.forLanguageTags This comment sounds confusing. LocaleList.forLanguageTags converts comma separated BCP 47 strings but this function can be used only for the single locale. Maybe we should refer Locale.forLanguageTag here? https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:159: * Converts LocaleList object to the BCP 47 compliant string format. BCP 47 is for single locale representation. So, we should say Converts LocaleList object to the comma separated BCP 47 compliant string format. https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:167: Locale locale; Please remove this line and change the L.169 as Locale locale = getUpdatedLocaleForChromium(localeList.get(i)); https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:188: * @return The default country code set during install. If the default is a LocaleList, return "If the default is a LocaleList" sounds weird to me. How about If the API level is 24 or newer, returns the country code of the first locale. BTW, is the first locale of the LocaleList.getDefault is the expected locale here? Note that there are two APIs for getting default locales, LocaleList.getDefault() and LocaleList.getAdjustedDefault(). The first locale with the latter API is safe for using UI language but the former API is not. https://codereview.chromium.org/2481293004/diff/20001/base/android/javatests/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/20001/base/android/javatests/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:10: import android.test.suitebuilder.annotation.SmallTest; Looks like android.test.suitebuilder.annotation.SmallTest is deprecated. Can you use android.support.test.filters.SmallTest instead? Please ignore my comment if the Chromium is not ready for the new testing library. https://developer.android.com/reference/android/test/suitebuilder/annotation/...
The CQ bit was checked by yirui@google.com to run a CQ dry run
Patchset #3 (id:40001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/1/base/android/javatests/src/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:127: localeList = new LocaleList(locale1); On 2016/11/08 17:26:41, Seigo Nonaka wrote: > On 2016/11/08 10:39:31, yirui wrote: > > On 2016/11/08 09:31:11, Seigo Nonaka wrote: > > > Looks like this test doesn't work on M and before. > > > > Yes, this is only for Android N. For Android M and before, toLanguageTag is > > tests in testToLanguageTag. > > I meant this test also runs on Android M and before, so you need to check API > version or you need to put MinAndroidSdkLevel annotation. > e.g. > https://cs.chromium.org/chromium/src/content/public/android/javatests/src/org... Thanks for the information. I think now I can also test getUpdatedLocaleForAndroid/getUpdatedLocaleForChromium since these two also require a minimum SDK level. https://codereview.chromium.org/2481293004/diff/20001/android_webview/java/sr... File android_webview/java/src/org/chromium/android_webview/AwContents.java (right): https://codereview.chromium.org/2481293004/diff/20001/android_webview/java/sr... android_webview/java/src/org/chromium/android_webview/AwContents.java:1013: sCurrentLocale = locales; On 2016/11/08 17:26:41, Seigo Nonaka wrote: > sCurrentLocales as well? Done. https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:125: * This works for API is lower than 24. For API is 24 and above, LocaleList.forLanguageTags On 2016/11/08 17:26:41, Seigo Nonaka wrote: > This comment sounds confusing. LocaleList.forLanguageTags converts comma > separated BCP 47 strings but this function can be used only for the single > locale. > Maybe we should refer Locale.forLanguageTag here? As discussed offline, removed this comment. https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:159: * Converts LocaleList object to the BCP 47 compliant string format. On 2016/11/08 17:26:41, Seigo Nonaka wrote: > BCP 47 is for single locale representation. So, we should say > > Converts LocaleList object to the comma separated BCP 47 compliant string > format. Done. https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:167: Locale locale; On 2016/11/08 17:26:41, Seigo Nonaka wrote: > Please remove this line and change the L.169 as > > Locale locale = getUpdatedLocaleForChromium(localeList.get(i)); Done. https://codereview.chromium.org/2481293004/diff/20001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:188: * @return The default country code set during install. If the default is a LocaleList, return On 2016/11/08 17:26:41, Seigo Nonaka wrote: > "If the default is a LocaleList" sounds weird to me. > How about > > If the API level is 24 or newer, returns the country code of the first locale. > > BTW, is the first locale of the LocaleList.getDefault is the expected locale > here? > Note that there are two APIs for getting default locales, > LocaleList.getDefault() and LocaleList.getAdjustedDefault(). > The first locale with the latter API is safe for using UI language but the > former API is not. As discussed offline, revert this part, the same reason as in ResourceExtractor. https://codereview.chromium.org/2481293004/diff/20001/base/android/javatests/... File base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java (right): https://codereview.chromium.org/2481293004/diff/20001/base/android/javatests/... base/android/javatests/src/org/chromium/base/LocaleUtilsTest.java:10: import android.test.suitebuilder.annotation.SmallTest; On 2016/11/08 17:26:42, Seigo Nonaka wrote: > Looks like android.test.suitebuilder.annotation.SmallTest is deprecated. > Can you use android.support.test.filters.SmallTest instead? > Please ignore my comment if the Chromium is not ready for the new testing > library. > > https://developer.android.com/reference/android/test/suitebuilder/annotation/... It may not be ready to use. The following message is shown: "error: package android.support.test.filters does not exist import android.support.test.filters.SmallTest;" also, android.support.test.InstrumentationRegistry should be used, since android.test.InstrumentationTestCase is deprecated in API 24 as well. But still, it is not ready for use. " error: package android.support.test does not exist import android.support.test.InstrumentationRegistry;"
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
please update patch description as well. https://codereview.chromium.org/2481293004/diff/60001/base/android/java/src/o... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/60001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:175: * @return a well-formed IETF BCP 47 language tag with language and country code that How about @return a comma separated language tags that represents a default locale list. Each language tag is well-formed IETF BCP 47 language tag with language and country code. We can regard a single locale as a special case of locale list.
Description was changed from ========== Add LocaleList in Android/Webview locale representation Add LocaleList related functions without break current behaviors with Locale and more tests for a better coverage of API levels. Relevant code is modified in Android Webview and Resource Extractor. Regarding to toLanguageTags and forLanguageTags, toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino). Whereas forLanguageTags in LocaleList, it converts languages correspondingly. Thus, an update for toLanguageTags is added and LocaleList.forLanguageTags is used for Android N and above. BUG=593515 ========== to ========== Add LocaleList in Android/Webview locale representation Add LocaleList related functions without break current behaviors with Locale and more tests for a better coverage of API levels. Relevant code is modified in Android Webview. Regarding to toLanguageTags and forLanguageTags, toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino). Whereas forLanguageTags in LocaleList, it converts languages correspondingly. Thus, an update for toLanguageTags is added and LocaleList.forLanguageTags is used for Android N and above. BUG=593515 ==========
The CQ bit was checked by yirui@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description of this CL is also modified. https://codereview.chromium.org/2481293004/diff/60001/base/android/java/src/o... File base/android/java/src/org/chromium/base/LocaleUtils.java (right): https://codereview.chromium.org/2481293004/diff/60001/base/android/java/src/o... base/android/java/src/org/chromium/base/LocaleUtils.java:175: * @return a well-formed IETF BCP 47 language tag with language and country code that On 2016/11/09 05:58:04, Seigo Nonaka wrote: > How about > > @return a comma separated language tags that represents a default locale list. > Each language tag is well-formed IETF BCP 47 language tag with language and > country code. > > We can regard a single locale as a special case of locale list. Done. That's true, what about modified as " @return a comma separated language tags string that represents a default locale or locales. Each language tag is well-formed IETF BCP 47 language tag with language and country code."
Description was changed from ========== Add LocaleList in Android/Webview locale representation Add LocaleList related functions without break current behaviors with Locale and more tests for a better coverage of API levels. Relevant code is modified in Android Webview. Regarding to toLanguageTags and forLanguageTags, toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino). Whereas forLanguageTags in LocaleList, it converts languages correspondingly. Thus, an update for toLanguageTags is added and LocaleList.forLanguageTags is used for Android N and above. BUG=593515 ========== to ========== Use LocaleList in Android/Webview locale representation From Android N, uses are able to select multiple languages in system Settings. This CL provides supports for multi-locales features for Chrome and System WebView. This does not breaking behaviours on Andriod M and before. Since toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino), an update for toLanguageTags is added. BUG=593515 ==========
lgtm
lgtm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
yirui@google.com changed reviewers: + torne@chromium.org
+torne@ as an owner. This CL provides supports for LocaleList in Android N. Please take a look at android_webview, base/android changes. Thanks in advance.
android_webview LGTM, thanks!
The CQ bit was checked by yirui@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Use LocaleList in Android/Webview locale representation From Android N, uses are able to select multiple languages in system Settings. This CL provides supports for multi-locales features for Chrome and System WebView. This does not breaking behaviours on Andriod M and before. Since toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino), an update for toLanguageTags is added. BUG=593515 ========== to ========== Use LocaleList in Android/Webview locale representation From Android N, uses are able to select multiple languages in system Settings. This CL provides supports for multi-locales features for Chrome and System WebView. This does not breaking behaviours on Andriod M and before. Since toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino), an update for toLanguageTags is added. BUG=593515 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Use LocaleList in Android/Webview locale representation From Android N, uses are able to select multiple languages in system Settings. This CL provides supports for multi-locales features for Chrome and System WebView. This does not breaking behaviours on Andriod M and before. Since toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino), an update for toLanguageTags is added. BUG=593515 ========== to ========== Use LocaleList in Android/Webview locale representation From Android N, uses are able to select multiple languages in system Settings. This CL provides supports for multi-locales features for Chrome and System WebView. This does not breaking behaviours on Andriod M and before. Since toLanguageTags in LocaleList does not convert Android language "tl"(Tagalog) to Chromium language "fil"(Filipino), an update for toLanguageTags is added. BUG=593515 Committed: https://crrev.com/9112d68d3d59f500ecd45e128337576aedaf5e64 Cr-Commit-Position: refs/heads/master@{#431128} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/9112d68d3d59f500ecd45e128337576aedaf5e64 Cr-Commit-Position: refs/heads/master@{#431128} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
