Skip to content

GitLab

T
tangled
Commit d3d16ac7 authored by miu@chromium.org's avatar miu@chromium.org
Browse files
Options

Revert of ChromeOS: should not show "Language changed" notification for... 

Revert of ChromeOS: should not show "Language changed" notification for certain languages. (patchset #6 of https://codereview.chromium.org/382973002/)

Reason for revert:
Link failure of browser_tests on ChromiumOS build bot:

http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=Linux%20ChromiumOS%20Builder%20%28dbg%29&number=50192


Original issue's description:
> ChromeOS: should not show "Language changed" notification for certain languages.
> 
> BUG=317718
> TEST=manually tested
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289985

NOTREECHECKS=true
NOTRY=true
TBR=alemate@chromium.org, nkostylev@chromium.org, jshin@chromium.org

Review URL: https://codereview.chromium.org/471403005

Cr-Commit-Position: refs/heads/master@{#290017}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290017 0039d316-1c4b-4281-b951-d872f2087c98
parent 1c28b793
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 308 deletions
chrome/browser/chromeos/locale_change_guard.cc
View file @ d3d16ac7
......@@ -4,8 +4,6 @@
#include "chrome/browser/chromeos/locale_change_guard.h"
#include <algorithm>
#include "ash/shell.h"
#include "ash/system/tray/system_tray.h"
#include "ash/system/tray/system_tray_notifier.h"
......@@ -35,17 +33,6 @@ using content::WebContents;
namespace chromeos {
namespace {
// This is the list of languages that do not require user notification when
// locale is switched automatically between regions within the same language.
//
// New language in kAcceptLanguageList should be added either here or to
// to the exception list in unit test.
const char* const kSkipShowNotificationLanguages[4] = {"en", "de", "fr", "it"};
} // anonymous namespace
LocaleChangeGuard::LocaleChangeGuard(Profile* profile)
: profile_(profile),
reverted_(false),
......@@ -166,11 +153,7 @@ void LocaleChangeGuard::Check() {
if (prefs->GetString(prefs::kApplicationLocaleAccepted) == to_locale)
return; // Already accepted.
// Locale change detected.
if (!ShouldShowLocaleChangeNotification(from_locale, to_locale))
return;
// Showing notification.
// Locale change detected, showing notification.
if (from_locale_ != from_locale || to_locale_ != to_locale) {
// Falling back to showing message in current locale.
LOG(ERROR) <<
......@@ -229,35 +212,4 @@ void LocaleChangeGuard::PrepareChangingLocale(
}
}
// static
bool LocaleChangeGuard::ShouldShowLocaleChangeNotification(
const std::string& from_locale,
const std::string& to_locale) {
const std::string from_lang = l10n_util::GetLanguage(from_locale);
const std::string to_lang = l10n_util::GetLanguage(to_locale);
if (from_locale == to_locale)
return false;
if (from_lang != to_lang)
return true;
const char* const* begin = kSkipShowNotificationLanguages;
const char* const* end = kSkipShowNotificationLanguages +
arraysize(kSkipShowNotificationLanguages);
return std::find(begin, end, from_lang) == end;
}
// static
const char* const*
LocaleChangeGuard::GetSkipShowNotificationLanguagesForTesting() {
return kSkipShowNotificationLanguages;
}
// static
size_t LocaleChangeGuard::GetSkipShowNotificationLanguagesSizeForTesting() {
return arraysize(kSkipShowNotificationLanguages);
}
} // namespace chromeos
chrome/browser/chromeos/locale_change_guard.h
View file @ d3d16ac7
......@@ -9,7 +9,6 @@
#include "ash/system/locale/locale_observer.h"
#include "base/compiler_specific.h"
#include "base/gtest_prod_util.h"
#include "base/lazy_instance.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
......@@ -48,11 +47,6 @@ class LocaleChangeGuard : public content::NotificationObserver,
void OnLogin();
private:
FRIEND_TEST_ALL_PREFIXES(LocaleChangeGuardTest,
ShowNotificationLocaleChanged);
FRIEND_TEST_ALL_PREFIXES(LocaleChangeGuardTest,
ShowNotificationLocaleChangedList);
void RevertLocaleChangeCallback(const base::ListValue* list);
void Check();
......@@ -61,13 +55,6 @@ class LocaleChangeGuard : public content::NotificationObserver,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
// Returns true if we should notify user about automatic locale change.
static bool ShouldShowLocaleChangeNotification(const std::string& from_locale,
const std::string& to_locale);
static const char* const* GetSkipShowNotificationLanguagesForTesting();
static size_t GetSkipShowNotificationLanguagesSizeForTesting();
std::string from_locale_;
std::string to_locale_;
Profile* profile_;
......
chrome/browser/chromeos/locale_change_guard_unittest.cc deleted 100644 → 0
View file @ 1c28b793
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/chromeos/locale_change_guard.h"
#include <string.h>
#include "base/macros.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
namespace {
// These languages require user notification when locale is automatically
// switched between different regions within the same language.
const char* const kShowNotificationLanguages[] = {
"af", // Afrikaans
"am", // Amharic
"ar", // Arabic
"az", // Azerbaijani
"be", // Belarusian
"bg", // Bulgarian
"bh", // Bihari
"bn", // Bengali
"br", // Breton
"bs", // Bosnian
"ca", // Catalan
"co", // Corsican
"cs", // Czech
"cy", // Welsh
"da", // Danish
"el", // Greek
"eo", // Esperanto
"es", // Spanish
"et", // Estonian
"eu", // Basque
"fa", // Persian
"fi", // Finnish
"fil", // Filipino
"fo", // Faroese
"fy", // Frisian
"ga", // Irish
"gd", // Scots Gaelic
"gl", // Galician
"gn", // Guarani
"gu", // Gujarati
"ha", // Hausa
"haw", // Hawaiian
"he", // Hebrew
"hi", // Hindi
"hr", // Croatian
"hu", // Hungarian
"hy", // Armenian
"ia", // Interlingua
"id", // Indonesian
"is", // Icelandic
"ja", // Japanese
"jw", // Javanese
"ka", // Georgian
"kk", // Kazakh
"km", // Cambodian
"kn", // Kannada
"ko", // Korean
"ku", // Kurdish
"ky", // Kyrgyz
"la", // Latin
"ln", // Lingala
"lo", // Laothian
"lt", // Lithuanian
"lv", // Latvian
"mk", // Macedonian
"ml", // Malayalam
"mn", // Mongolian
"mo", // Moldavian
"mr", // Marathi
"ms", // Malay
"mt", // Maltese
"nb", // Norwegian (Bokmal)
"ne", // Nepali
"nl", // Dutch
"nn", // Norwegian (Nynorsk)
"no", // Norwegian
"oc", // Occitan
"om", // Oromo
"or", // Oriya
"pa", // Punjabi
"pl", // Polish
"ps", // Pashto
"pt", // Portuguese
"qu", // Quechua
"rm", // Romansh
"ro", // Romanian
"ru", // Russian
"sd", // Sindhi
"sh", // Serbo-Croatian
"si", // Sinhalese
"sk", // Slovak
"sl", // Slovenian
"sn", // Shona
"so", // Somali
"sq", // Albanian
"sr", // Serbian
"st", // Sesotho
"su", // Sundanese
"sv", // Swedish
"sw", // Swahili
"ta", // Tamil
"te", // Telugu
"tg", // Tajik
"th", // Thai
"ti", // Tigrinya
"tk", // Turkmen
"to", // Tonga
"tr", // Turkish
"tt", // Tatar
"tw", // Twi
"ug", // Uighur
"uk", // Ukrainian
"ur", // Urdu
"uz", // Uzbek
"vi", // Vietnamese
"xh", // Xhosa
"yi", // Yiddish
"yo", // Yoruba
"zh", // Chinese
"zu", // Zulu
};
} // anonymous namespace
namespace chromeos {
TEST(LocaleChangeGuardTest, ShowNotificationLocaleChanged) {
// "en" is used as "global default" in many places.
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "it"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("it", "en"));
// Between two latin locales.
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "it"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("it", "fr"));
// en <-> non-latin locale
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "zh"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "en"));
// latin <-> non-latin locale
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "zh"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "fr"));
// same language
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "fr"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "zh"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en-US"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-GB", "en-US"));
// Different regions within the same language
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en-au"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-AU", "en"));
EXPECT_FALSE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-AU", "en-GB"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "zh-CN"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh-CN", "zh-TW"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("es", "es-419"));
EXPECT_TRUE(
LocaleChangeGuard::ShouldShowLocaleChangeNotification("es", "es-ES"));
}
TEST(LocaleChangeGuardTest, ShowNotificationLocaleChangedList) {
for (size_t i = 0; i < l10n_util::GetAcceptLanguageListSizeForTesting();
++i) {
const char* const locale = l10n_util::GetAcceptLanguageListForTesting()[i];
const char* const dash = strchr(locale, '-');
const std::string language =
(dash ? std::string(locale, dash - locale) : std::string(locale));
const char* const* allowed_begin = kShowNotificationLanguages;
const char* const* allowed_end =
kShowNotificationLanguages + arraysize(kShowNotificationLanguages);
const bool notification_allowed =
(std::find(allowed_begin, allowed_end, language) != allowed_end);
const char* const* skipped_begin =
LocaleChangeGuard::GetSkipShowNotificationLanguagesForTesting();
const char* const* skipped_end =
skipped_begin +
LocaleChangeGuard::GetSkipShowNotificationLanguagesSizeForTesting();
const bool notification_skipped =
(std::find(skipped_begin, skipped_end, language) != skipped_end);
EXPECT_TRUE(notification_allowed ^ notification_skipped)
<< "Language '" << language << "' (from locale '" << locale
<< "') must be in exactly one list: either "
"kSkipShowNotificationLanguages (found=" << notification_skipped
<< ") or kShowNotificationLanguages (found=" << notification_allowed
<< ").";
}
}
} // namespace chromeos
chrome/browser/ui/webui/chromeos/login/l10n_util.cc
View file @ d3d16ac7
......@@ -126,10 +126,10 @@ scoped_ptr<base::ListValue> GetLanguageList(
it != language_index.end(); ++it) {
const std::string& language_id = it->first;
const std::string lang = l10n_util::GetLanguage(language_id);
const size_t dash_pos = language_id.find_first_of('-');
// Ignore non-specific codes.
if (lang.empty() || lang == language_id)
if (dash_pos == std::string::npos || dash_pos == 0)
continue;
if (std::find(base_language_codes.begin(),
......
chrome/chrome_tests_unit.gypi
View file @ d3d16ac7
......@@ -737,7 +737,6 @@
'browser/chromeos/input_method/input_method_util_unittest.cc',
'browser/chromeos/kiosk_mode/kiosk_mode_idle_logout_unittest.cc',
'browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc',
'browser/chromeos/locale_change_guard_unittest.cc',
'browser/chromeos/login/auth/online_attempt_unittest.cc',
'browser/chromeos/login/auth/parallel_authenticator_unittest.cc',
'browser/chromeos/login/existing_user_controller_auto_login_unittest.cc',
......
ui/base/l10n/l10n_util.cc
View file @ d3d16ac7
......@@ -311,11 +311,6 @@ std::string GetCanonicalLocale(const std::string& locale) {
return base::i18n::GetCanonicalLocale(locale.c_str());
}
std::string GetLanguage(const std::string& locale) {
const std::string::size_type hyphen_pos = locale.find('-');
return std::string(locale, 0, hyphen_pos);
}
bool CheckAndResolveLocale(const std::string& locale,
std::string* resolved_locale) {
#if defined(OS_MACOSX)
......@@ -339,9 +334,10 @@ bool CheckAndResolveLocale(const std::string& locale,
// does not support but available on Windows. We fall
// back to en-US in GetApplicationLocale so that it's a not critical,
// but we can do better.
const std::string lang(GetLanguage(locale));
if (lang.size() < locale.size()) {
std::string region(locale, lang.size() + 1);
std::string::size_type hyphen_pos = locale.find('-');
std::string lang(locale, 0, hyphen_pos);
if (hyphen_pos != std::string::npos && hyphen_pos > 0) {
std::string region(locale, hyphen_pos + 1);
std::string tmp_locale(lang);
// Map es-RR other than es-ES to es-419 (Chrome's Latin American
// Spanish locale).
......@@ -881,12 +877,4 @@ int GetLocalizedContentsWidthInPixels(int pixel_resource_id) {
return width;
}
const char* const* GetAcceptLanguageListForTesting() {
return kAcceptLanguageList;
}
size_t GetAcceptLanguageListSizeForTesting() {
return arraysize(kAcceptLanguageList);
}
} // namespace l10n_util
ui/base/l10n/l10n_util.h
View file @ d3d16ac7
......@@ -24,9 +24,6 @@ namespace l10n_util {
// std::string as an argument.
UI_BASE_EXPORT std::string GetCanonicalLocale(const std::string& locale);
// Takes normalized locale as |locale|. Returns language part (before '-').
UI_BASE_EXPORT std::string GetLanguage(const std::string& locale);
// This method translates a generic locale name to one of the locally defined
// ones. This method returns true if it succeeds.
UI_BASE_EXPORT bool CheckAndResolveLocale(const std::string& locale,
......@@ -188,10 +185,6 @@ UI_BASE_EXPORT void GetAcceptLanguagesForLocale(
// designer given constraints which might dependent on the language used.
UI_BASE_EXPORT int GetLocalizedContentsWidthInPixels(int pixel_resource_id);
const char* const* GetAcceptLanguageListForTesting();
size_t GetAcceptLanguageListSizeForTesting();
} // namespace l10n_util
#endif // UI_BASE_L10N_L10N_UTIL_H_
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