Always display the input method name for Japanese.

In the language menu, we show language names only if there are no
ambiguity. However, showing "Japanese" is confusing, when only
Japanese keyboard layout is enabled. Users would expect they can
enter Japanese with an input method, rather than just using
"Japanese keyboard".

BUG=chromium-os:5675
TEST=unit tests; manually on the netbook

Review URL: http://codereview.chromium.org/3122019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56190 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 82 insertions, 17 deletions

diff --git a/chrome/browser/chromeos/status/language_menu_button.cc b/chrome/browser/chromeos/status/language_menu_button.cc
index c31e8b5..fe2518a 100644
--- a/chrome/browser/chromeos/status/language_menu_button.cc
+++ b/chrome/browser/chromeos/status/language_menu_button.cc
@@ -322,7 +322,8 @@ string16 LanguageMenuButton::GetLabelAt(int index) const {
     const std::string language_code =
         input_method::GetLanguageCodeFromDescriptor(
             input_method_descriptors_->at(index));
-    bool need_method_name = (need_method_name_.count(language_code) > 0);
+    const bool need_method_name =
+        (ambiguous_language_code_set_.count(language_code) > 0);
     name = GetTextForMenu(input_method_descriptors_->at(index),
                           need_method_name);
   } else if (GetPropertyIndex(index, &index)) {
@@ -486,26 +487,17 @@ void LanguageMenuButton::RebuildModel() {
   // Indicates if separator's needed before each section.
   bool need_separator = false;
 
-  need_method_name_.clear();
-  std::set<std::string> languages_seen;
+  ambiguous_language_code_set_.clear();
   if (!input_method_descriptors_->empty()) {
     // We "abuse" the command_id and group_id arguments of AddRadioItem method.
     // A COMMAND_ID_XXX enum value is passed as command_id, and array index of
     // |input_method_descriptors_| or |property_list| is passed as group_id.
     for (size_t i = 0; i < input_method_descriptors_->size(); ++i) {
       model_->AddRadioItem(COMMAND_ID_INPUT_METHODS, dummy_label, i);
-
-      const std::string language_code
-          = input_method::GetLanguageCodeFromDescriptor(
-              input_method_descriptors_->at(i));
-      // If there is more than one input method for this language, then we need
-      // to display the method name.
-      if (languages_seen.find(language_code) == languages_seen.end()) {
-        languages_seen.insert(language_code);
-      } else {
-        need_method_name_.insert(language_code);
-      }
     }
+
+    GetAmbiguousLanguageCodeSet(*input_method_descriptors_,
+                                &ambiguous_language_code_set_);
     need_separator = true;
   }
 
@@ -648,6 +640,31 @@ void LanguageMenuButton::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterStringPref(kPreferredKeyboardLayout, "");
 }
 
+void LanguageMenuButton::GetAmbiguousLanguageCodeSet(
+    const InputMethodDescriptors& input_method_descriptors,
+    std::set<std::string>* ambiguous_language_code_set) {
+  DCHECK(ambiguous_language_code_set);
+  ambiguous_language_code_set->clear();
+
+  std::set<std::string> languages_seen;
+  for (size_t i = 0; i < input_method_descriptors.size(); ++i) {
+    const std::string language_code
+        = input_method::GetLanguageCodeFromDescriptor(
+            input_method_descriptors.at(i));
+    // If there is more than one input method for this language, then we need
+    // to display the method name.
+    if (languages_seen.count(language_code) > 0 ||
+        // Special-case Japanese as showing the language name alone is
+        // confusing when Japanese keyboard is enabled but Japanese
+        // input methods are not.
+        language_code == "ja") {
+      ambiguous_language_code_set->insert(language_code);
+    } else {
+      languages_seen.insert(language_code);
+    }
+  }
+}
+
 void LanguageMenuButton::Observe(NotificationType type,
                                  const NotificationSource& source,
                                  const NotificationDetails& details) {
diff --git a/chrome/browser/chromeos/status/language_menu_button.h b/chrome/browser/chromeos/status/language_menu_button.h
index abc694d..991578f 100644
--- a/chrome/browser/chromeos/status/language_menu_button.h
+++ b/chrome/browser/chromeos/status/language_menu_button.h
@@ -76,6 +76,14 @@ class LanguageMenuButton : public views::MenuButton,
   // Registers input method preferences for the login screen.
   static void RegisterPrefs(PrefService* local_state);
 
+  // Gets the language codes that are ambiguous if we only show the
+  // language names in the menu. For these languages, we'll show the input
+  // method names in addition to the language names. The original contents
+  // of |ambiguous_language_code_set| are lost.
+  static void GetAmbiguousLanguageCodeSet(
+      const InputMethodDescriptors& input_method_descriptors,
+      std::set<std::string>* ambiguous_language_code_set);
+
  protected:
   // views::View implementation.
   virtual void OnLocaleChanged();
@@ -115,8 +123,9 @@ class LanguageMenuButton : public views::MenuButton,
   StringPrefMember previous_input_method_pref_;
   StringPrefMember current_input_method_pref_;
 
-  // Languages that need the input method name displayed.
-  std::set<std::string> need_method_name_;
+  // Language codes that can be ambiguous. See comments at
+  // GetAmbiguousLanguageCodeSet() for details.
+  std::set<std::string> ambiguous_language_code_set_;
 
   // We borrow menus::SimpleMenuModel implementation to maintain the current
   // content of the pop-up menu. The menus::MenuModel is implemented using this
diff --git a/chrome/browser/chromeos/status/language_menu_button_unittest.cc b/chrome/browser/chromeos/status/language_menu_button_unittest.cc
index b769ad7..87cdc36 100644
--- a/chrome/browser/chromeos/status/language_menu_button_unittest.cc
+++ b/chrome/browser/chromeos/status/language_menu_button_unittest.cc
@@ -30,7 +30,7 @@ TEST(LanguageMenuButtonTest, GetTextForIndicatorTest) {
 
   // Test special cases.
   {
-    InputMethodDescriptor desc("xkb:us:dvorak:eng", "Dvorak", "us", "us");
+    InputMethodDescriptor desc("xkb:us:dvorak:eng", "Dvorak", "us", "eng");
     EXPECT_EQ(L"DV", LanguageMenuButton::GetTextForIndicator(desc));
   }
   {
@@ -86,4 +86,43 @@ TEST(LanguageMenuButtonTest, GetTextForTooltipTest) {
   }
 }
 
+TEST(LanguageMenuButtonTest, GetAmbiguousLanguageCodeSet) {
+  {
+    std::set<std::string> ambiguous_language_code_set;
+    InputMethodDescriptors descriptors;
+    descriptors.push_back(InputMethodDescriptor(
+        "xkb:us::eng", "USA", "us", "eng"));
+    LanguageMenuButton::GetAmbiguousLanguageCodeSet(
+        descriptors, &ambiguous_language_code_set);
+    // There is no ambituity.
+    EXPECT_TRUE(ambiguous_language_code_set.empty());
+  }
+  {
+    std::set<std::string> ambiguous_language_code_set;
+    InputMethodDescriptors descriptors;
+    descriptors.push_back(InputMethodDescriptor(
+        "xkb:us::eng", "USA", "us", "eng"));
+    descriptors.push_back(InputMethodDescriptor(
+        "xkb:us:dvorak:eng", "Dvorak", "us", "eng"));
+    LanguageMenuButton::GetAmbiguousLanguageCodeSet(
+        descriptors, &ambiguous_language_code_set);
+    // This is ambiguous, as two input methods are present for "en-US".
+    EXPECT_EQ(1U, ambiguous_language_code_set.size());
+    EXPECT_EQ(1U, ambiguous_language_code_set.count("en-US"));
+  }
+  {
+    std::set<std::string> ambiguous_language_code_set;
+    InputMethodDescriptors descriptors;
+    descriptors.push_back(InputMethodDescriptor(
+        "xkb:jp::jpn", "Japan", "jp", "jpn"));
+    LanguageMenuButton::GetAmbiguousLanguageCodeSet(
+        descriptors, &ambiguous_language_code_set);
+    // Japanese is special. Showing the language name alone for the
+    // Japanese keyboard layout is confusing, hence we consider it
+    // ambiguous.
+    EXPECT_EQ(1U, ambiguous_language_code_set.size());
+    EXPECT_EQ(1U, ambiguous_language_code_set.count("ja"));
+  }
+}
+
 }  // namespace chromeos