Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Revert of ChromeOS: should not show "Language changed" notification f…
Browse files Browse the repository at this point in the history
…or 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
[email protected], [email protected], [email protected]

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
  • Loading branch information
[email protected] committed Aug 15, 2014
1 parent 1c28b79 commit d3d16ac
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 308 deletions.
50 changes: 1 addition & 49 deletions chrome/browser/chromeos/locale_change_guard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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) <<
Expand Down Expand Up @@ -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
13 changes: 0 additions & 13 deletions chrome/browser/chromeos/locale_change_guard.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();

Expand All @@ -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_;
Expand Down
220 changes: 0 additions & 220 deletions chrome/browser/chromeos/locale_change_guard_unittest.cc

This file was deleted.

4 changes: 2 additions & 2 deletions chrome/browser/ui/webui/chromeos/login/l10n_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
20 changes: 4 additions & 16 deletions ui/base/l10n/l10n_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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).
Expand Down Expand Up @@ -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
Loading

0 comments on commit d3d16ac

Please sign in to comment.