Skip to content

GitLab

T
tangled
Commit ce0ec6fc authored by Michael Giuffrida's avatar Michael Giuffrida Committed by Commit Bot
Browse files
Options

Partial revert of "Use preferred language in extension i18n" 

This is a logical revert of "Use preferred language in extension i18n"
(commit 693d01a6). Because of some
intervening changes causing more complicated merges, it's simpler to
just undo the side effects of the original change, leaving the changed
boilerplate intact.

Reason for revert: Testing perf regression in crbug.com/898191

If this exonerates the original CL, we will revert this change.
Otherwise, we will clean up the boilerplate added for the original CL.

Original change's description:
> Use preferred language in extension i18n
>
> When localizing an extension, prioritize the user's preferred language
> over the application locale.
>
> Normally, these are the same. In some cases, they differ. For example,
> the user may choose to display Chrome in "en-CA" (the intl.app_locale
> pref), but because we don't have translations for that, the actual
> UI locale is "en-GB".
>
> This CL makes extensions try to use the user's preferred locale for i18n
> ("en_CA"), falling back to the UI locale ("en_GB") if no locale
> directory is found for the preferred locale.
>
> This change also updates the "current_locale" manifest key to reflect
> the user's preferred locale, or the UI locale if intl.app_locale is
> unset.
>
> Note: Like the application locale, the preferred locale is only set at
> startup (and in certain situations in CrOS). If the user changes their
> intl.app_locale pref, they have to restart Chrome before extensions are
> re-localized, just like before.
>
> Bug: 874225
> Change-Id: I1aabe3c3680b77d6522193e764aec15a3d618d2d
> Reviewed-on: https://chromium-review.googlesource.com/c/1244666
> Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600619}

Bug: 898191
Change-Id: Iaddceb16d61ffc5572a25ed31276443ea5b5baa4
Reviewed-on: https://chromium-review.googlesource.com/c/1308765Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604819}
parent 10912910
Hide whitespace changes
Inline Side-by-side
Showing with 17 additions and 72 deletions
extensions/common/extension_l10n_util.cc
View file @ ce0ec6fc
......@@ -114,10 +114,8 @@ std::string& GetPreferredLocale() {
// Returns the desired locale to use for localization.
std::string LocaleForLocalization() {
std::string preferred_locale =
l10n_util::NormalizeLocale(GetPreferredLocale());
if (!preferred_locale.empty())
return preferred_locale;
// TODO(michaelpg): Check preferred_locale first. That change was reverted to
// check if it alleviated: https://crbug.com/898191.
return extension_l10n_util::CurrentLocaleOrDefault();
}
......@@ -351,17 +349,8 @@ void GetAllFallbackLocales(const std::string& application_locale,
const std::string& default_locale,
std::vector<std::string>* all_fallback_locales) {
DCHECK(all_fallback_locales);
// Use the preferred locale if available. Otherwise, fall back to the
// application locale or the application locale's parent locales. Thus, a
// preferred locale of "en_CA" with an application locale of "en_GB" will
// first try to use an en_CA locale folder, followed by en_GB, followed by en.
std::string preferred_locale =
l10n_util::NormalizeLocale(GetPreferredLocale());
if (!preferred_locale.empty() && preferred_locale != default_locale &&
preferred_locale != application_locale) {
all_fallback_locales->push_back(preferred_locale);
}
// TODO(michaelpg): Check preferred_locale first. That change was reverted to
// check if it alleviated: https://crbug.com/898191.
if (!application_locale.empty() && application_locale != default_locale)
l10n_util::GetParentLocales(application_locale, all_fallback_locales);
all_fallback_locales->push_back(default_locale);
......
extensions/common/extension_l10n_util_unittest.cc
View file @ ce0ec6fc
......@@ -124,7 +124,6 @@ TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocalesAndMessagesFile) {
}
TEST(ExtensionL10nUtil, LoadMessageCatalogsValidFallback) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
base::FilePath install_dir;
ASSERT_TRUE(base::PathService::Get(DIR_TEST_DATA, &install_dir));
install_dir =
......@@ -141,7 +140,6 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsValidFallback) {
}
TEST(ExtensionL10nUtil, LoadMessageCatalogsMissingFiles) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
base::ScopedTempDir temp;
ASSERT_TRUE(temp.CreateUniqueTempDir());
......@@ -157,7 +155,6 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsMissingFiles) {
}
TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
base::ScopedTempDir temp;
ASSERT_TRUE(temp.CreateUniqueTempDir());
......@@ -183,7 +180,6 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) {
}
TEST(ExtensionL10nUtil, LoadMessageCatalogsDuplicateKeys) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
base::ScopedTempDir temp;
ASSERT_TRUE(temp.CreateUniqueTempDir());
......@@ -603,97 +599,57 @@ TEST(ExtensionL10nUtil, LocalizeManifestWithSearchProviderMsgs) {
EXPECT_TRUE(error.empty());
}
// Tests that we don't relocalize with a null manifest.
// Try with NULL manifest.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithNullManifest) {
EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(NULL));
}
// Tests that we don't relocalize with default and current locales missing.
// Try with default and current locales missing.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestEmptyManifest) {
base::DictionaryValue manifest;
EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we relocalize without a current locale.
// Try with missing current_locale.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithDefaultLocale) {
base::DictionaryValue manifest;
manifest.SetString(keys::kDefaultLocale, "en_US");
EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we don't relocalize without a default locale.
// Try with missing default_locale.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithCurrentLocale) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
base::DictionaryValue manifest;
manifest.SetString(keys::kCurrentLocale, "en_US");
manifest.SetString(keys::kCurrentLocale,
extension_l10n_util::CurrentLocaleOrDefault());
EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we don't relocalize with same current_locale as system locale.
// Try with all data present, but with same current_locale as system locale.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestSameCurrentLocale) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
base::DictionaryValue manifest;
manifest.SetString(keys::kDefaultLocale, "en_US");
manifest.SetString(keys::kCurrentLocale, "en_US");
manifest.SetString(keys::kCurrentLocale,
extension_l10n_util::CurrentLocaleOrDefault());
EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we relocalize with a different current_locale.
// Try with all data present, but with different current_locale.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestDifferentCurrentLocale) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
base::DictionaryValue manifest;
manifest.SetString(keys::kDefaultLocale, "en_US");
manifest.SetString(keys::kCurrentLocale, "sr");
EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we don't relocalize with the same current_locale as preferred
// locale.
TEST(ExtensionL10nUtil, ShouldRelocalizeManifestSameCurrentLocaleAsPreferred) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
base::DictionaryValue manifest;
manifest.SetString(keys::kDefaultLocale, "en_US");
manifest.SetString(keys::kCurrentLocale, "en_CA");
// Preferred and current locale are both en_CA.
EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
// Tests that we relocalize with a different current_locale from the preferred
// locale.
TEST(ExtensionL10nUtil,
ShouldRelocalizeManifestDifferentCurrentLocaleThanPreferred) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
base::DictionaryValue manifest;
manifest.SetString(keys::kDefaultLocale, "en_US");
manifest.SetString(keys::kCurrentLocale, "en_GB");
// Requires relocalization as the preferred (en_CA) differs from current
// (en_GB).
EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
}
TEST(ExtensionL10nUtil, GetAllFallbackLocales) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
std::vector<std::string> fallback_locales;
extension_l10n_util::GetAllFallbackLocales("en_US", "all", &fallback_locales);
ASSERT_EQ(3U, fallback_locales.size());
EXPECT_EQ("en_US", fallback_locales[0]);
EXPECT_EQ("en", fallback_locales[1]);
EXPECT_EQ("all", fallback_locales[2]);
}
TEST(ExtensionL10nUtil, GetAllFallbackLocalesWithPreferredLocale) {
extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
std::vector<std::string> fallback_locales;
extension_l10n_util::GetAllFallbackLocales("en_GB", "all", &fallback_locales);
ASSERT_EQ(4U, fallback_locales.size());
EXPECT_EQ("en_CA", fallback_locales[0]);
EXPECT_EQ("en_GB", fallback_locales[1]);
EXPECT_EQ("en", fallback_locales[2]);
EXPECT_EQ("all", fallback_locales[3]);
CHECK_EQ("en_US", fallback_locales[0]);
CHECK_EQ("en", fallback_locales[1]);
CHECK_EQ("all", fallback_locales[2]);
}
} // namespace
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment