Bug fix: Append a language to the list after blocking it for Translate on Chrome OS

chrome/browser/translate/translate_prefs.cc

Index: chrome/browser/translate/translate_prefs.cc
diff --git a/chrome/browser/translate/translate_prefs.cc b/chrome/browser/translate/translate_prefs.cc
index a6b51d930d01e30040955c2c152cc81f094a62f4..4cef11feef8e3aa62bdf75a9dc207322d9a54167 100644
--- a/chrome/browser/translate/translate_prefs.cc
+++ b/chrome/browser/translate/translate_prefs.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/translate/translate_prefs.h"
 
+#include <set>
+
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ -64,28 +66,34 @@ std::string ConvertLangCodeForTranslation(const std::string &lang) {
   return main_part;
 }
 
-}  // namespace
-
-namespace {
+// Expands language codes to make these more suitable for Accept-Language.
+// Example: ['en-US', 'ja', 'en-CA'] => ['en-US', 'en', 'ja', 'en-CA'].
+// 'en' won't appear twice as this function eliminates duplicates.
+void ExpandLanguageCodes(const std::vector<std::string>& languages,
+                         std::vector<std::string>* expanded_languages) {
+  DCHECK(expanded_languages);

[Takashi Toyoshima, 2013/09/09 12:26:11]

DCHECK(expanded_langauges->empty()) ?

[hajimehoshi, 2013/09/10 03:21:45]

Done.

 
-void AppendLanguageToAcceptLanguages(PrefService* prefs,
-                                     const std::string& language) {
-  if (!TranslateAcceptLanguages::CanBeAcceptLanguage(language))
-    return;
+  // used to eliminate duplicates.
+  std::set<std::string> seen;
 
-  std::string accept_language = language;
-  TranslateUtil::ToChromeLanguageSynonym(&accept_language);
+  for (std::vector<std::string>::const_iterator it = languages.begin();
+       it != languages.end(); ++it) {
+    const std::string& language = *it;
+    if (seen.find(language) == seen.end()) {
+      expanded_languages->push_back(language);
+      seen.insert(language);
+    }
 
-  std::string accept_languages_str = prefs->GetString(prefs::kAcceptLanguages);
-  std::vector<std::string> accept_languages;
-  base::SplitString(accept_languages_str, ',', &accept_languages);
-  if (std::find(accept_languages.begin(),
-                accept_languages.end(),
-                accept_language) == accept_languages.end()) {
-    accept_languages.push_back(accept_language);
+    std::vector<std::string> tokens;
+    base::SplitString(language, '-', &tokens);
+    if (tokens.size() == 0)
+      continue;
+    const std::string& main_part = tokens[0];
+    if (seen.find(main_part) == seen.end()) {
+      expanded_languages->push_back(main_part);
+      seen.insert(main_part);
+    }
   }
-  accept_languages_str = JoinString(accept_languages, ',');
-  prefs->SetString(prefs::kAcceptLanguages, accept_languages_str);
 }
 
 }  // namespace
@@ -105,7 +113,19 @@ bool TranslatePrefs::IsBlockedLanguage(
 void TranslatePrefs::BlockLanguage(
     const std::string& original_language) {
   BlacklistValue(kPrefTranslateBlockedLanguages, original_language);
-  AppendLanguageToAcceptLanguages(prefs_, original_language);
+
+  // Add the language to the language list at chrome://settings/languages.
+  std::string language = original_language;
+  TranslateUtil::ToChromeLanguageSynonym(&language);
+
+  std::vector<std::string> languages;
+  GetLanguageList(&languages);
+
+  if (std::find(languages.begin(), languages.end(), language) ==
+      languages.end()) {
+    languages.push_back(language);
+    UpdateLanguageList(languages);
+  }
 }
 
 void TranslatePrefs::UnblockLanguage(
@@ -238,6 +258,35 @@ void TranslatePrefs::ResetTranslationAcceptedCount(
   update.Get()->SetInteger(language, 0);
 }
 
+void TranslatePrefs::GetLanguageList(std::vector<std::string>* languages) {
+  DCHECK(languages);

[Takashi Toyoshima, 2013/09/09 12:26:11]

DCHECK(languages->empty()) ?

[hajimehoshi, 2013/09/10 03:21:45]

Done.

+
+#if defined(OS_CHROMEOS)
+  const char* key = prefs::kLanguagePreferredLanguages;
+#else
+  const char* key = prefs::kAcceptLanguages;
+#endif
+
+  std::string languages_str = prefs_->GetString(key);
+  base::SplitString(languages_str, ',', languages);
+}
+
+void TranslatePrefs::UpdateLanguageList(
+    const std::vector<std::string>& languages) {
+#if defined(OS_CHROMEOS)
+  std::string languages_str = JoinString(languages, ',');
+  prefs_->SetString(prefs::kLanguagePreferredLanguages, languages_str);
+#endif
+
+  // Save the same language list as accept languages preference as well, but we
+  // need to expand the language list, to make it more acceptable. For instance,
+  // some web sites don't understand 'en-US' but 'en'. See crosbug.com/9884.
+  std::vector<std::string> accept_languages;
+  ExpandLanguageCodes(languages, &accept_languages);

[Takashi Toyoshima, 2013/09/09 12:26:11]

I don't think this works as you expect.

In the case of Chrome OS, it works since it uses kLanguagePreferredLanguages to
get the language list for the preference. If you want to apply the same logic
here, you should update GetLanguageList to return kLanguagePreferredLanguages.
Of course, you need to copy kAcceptLanguages to kLanguagePreferredLanguages if
it doesn't exist.

Otherwise, unexpected languages will appear in the preference. E.g., what happen
if the languages contain "zh-CN", but "zh".

[hajimehoshi, 2013/09/10 03:21:45]

Sorry but I couldn't understand. Please discuss later.

+  std::string accept_languages_str = JoinString(accept_languages, ',');
+  prefs_->SetString(prefs::kAcceptLanguages, accept_languages_str);
+}
+
 // TranslatePrefs: public, static: ---------------------------------------------
 
 // static