Ignore NumLock modifier so that IME could be switched even if NumLock is enabled.

Remove Super and Hyper modifiers support for now. I thought Mod2Mask in X corresponds to Super and Mod3Mask does Hyper, but I was wrong. In the current X configuration for Chrome OS, Mod2Mask is for NumLock.

BUG=chromium-os:21842
TEST=ran unit_tests


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

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

2 files changed, 23 insertions, 21 deletions

diff --git a/chrome/browser/chromeos/input_method/hotkey_manager.cc b/chrome/browser/chromeos/input_method/hotkey_manager.cc
index 3ba9bd4..1244e7e 100644
--- a/chrome/browser/chromeos/input_method/hotkey_manager.cc
+++ b/chrome/browser/chromeos/input_method/hotkey_manager.cc
@@ -32,17 +32,15 @@ uint32 KeySymToModifier(KeySym keysym) {
       return Mod1Mask;
     case XK_Meta_R:
       return Mod1Mask;
-    case XK_Super_L:
-      return Mod2Mask;
-    case XK_Super_R:
-      return Mod2Mask;
-    case XK_Hyper_L:
-      return Mod3Mask;
-    case XK_Hyper_R:
-      return Mod3Mask;
     default:
       break;
   }
+
+  // Note for Mod[2-5]Mask: Usually, Mod2Mask corresponds to the Num Lock key,
+  // and Mod4Mask does the Windows key. However, to be exact, the assignment is
+  // dynamic, and could be changed in the future, I believe. FYI, xkeyboard.cc
+  // has code which dynamically obtains a mask for Num Lock.
+
   return 0;
 }
 
@@ -139,7 +137,7 @@ bool HotkeyManager::FilterKeyEvent(const XEvent& key_event) {
 
 uint32 HotkeyManager::NormalizeModifiers(
     uint32 keysym, uint32 modifiers, bool is_key_press) const {
-  modifiers &= (ControlMask | ShiftMask | Mod1Mask | Mod2Mask | Mod3Mask);
+  modifiers &= (ControlMask | ShiftMask | Mod1Mask);
   modifiers |= KeySymToModifier(keysym);
   if (!is_key_press) {
     modifiers |= kKeyReleaseMask;
diff --git a/chrome/browser/chromeos/input_method/hotkey_manager_unittest.cc b/chrome/browser/chromeos/input_method/hotkey_manager_unittest.cc
index 6b02455..13b9575 100644
--- a/chrome/browser/chromeos/input_method/hotkey_manager_unittest.cc
+++ b/chrome/browser/chromeos/input_method/hotkey_manager_unittest.cc
@@ -71,13 +71,15 @@ TEST(HotkeyManagerTest, TestIsModifier) {
   EXPECT_TRUE(manager.IsModifier(XK_Alt_R));
   EXPECT_TRUE(manager.IsModifier(XK_Meta_L));
   EXPECT_TRUE(manager.IsModifier(XK_Meta_R));
-  EXPECT_TRUE(manager.IsModifier(XK_Super_L));
-  EXPECT_TRUE(manager.IsModifier(XK_Super_R));
-  EXPECT_TRUE(manager.IsModifier(XK_Hyper_L));
-  EXPECT_TRUE(manager.IsModifier(XK_Hyper_R));
   EXPECT_FALSE(manager.IsModifier(XK_a));
   EXPECT_FALSE(manager.IsModifier(XK_A));
   EXPECT_FALSE(manager.IsModifier(XK_Tab));
+
+  // For now, we don't support Super and Hyper. See crosbug.com/21842.
+  EXPECT_FALSE(manager.IsModifier(XK_Super_L));
+  EXPECT_FALSE(manager.IsModifier(XK_Super_R));
+  EXPECT_FALSE(manager.IsModifier(XK_Hyper_L));
+  EXPECT_FALSE(manager.IsModifier(XK_Hyper_R));
 }
 
 TEST(HotkeyManagerTest, TestNormalizeModifiers) {
@@ -100,14 +102,6 @@ TEST(HotkeyManagerTest, TestNormalizeModifiers) {
             manager.NormalizeModifiers(XK_Meta_L, ShiftMask, true));
   EXPECT_EQ(Mod1Mask | ShiftMask | kReleaseMask,
             manager.NormalizeModifiers(XK_Meta_R, ShiftMask, false));
-  EXPECT_EQ(ShiftMask | Mod2Mask | 0x0U,
-            manager.NormalizeModifiers(XK_Super_L, ShiftMask, true));
-  EXPECT_EQ(Mod1Mask | Mod2Mask | kReleaseMask,
-            manager.NormalizeModifiers(XK_Super_R, Mod1Mask, false));
-  EXPECT_EQ(ShiftMask | Mod3Mask | kReleaseMask,
-            manager.NormalizeModifiers(XK_Hyper_L, ShiftMask, false));
-  EXPECT_EQ(Mod1Mask | Mod3Mask | 0x0U,
-            manager.NormalizeModifiers(XK_Hyper_R, Mod1Mask, true));
 
   // Test non modifier keys like XK_a.
   EXPECT_EQ(Mod1Mask | 0x0U,
@@ -120,6 +114,16 @@ TEST(HotkeyManagerTest, TestNormalizeModifiers) {
   EXPECT_EQ(Mod1Mask | ControlMask | kReleaseMask,
             manager.NormalizeModifiers(
                 XK_D, Mod1Mask | ControlMask, false));
+
+  // For now, we don't support Super and Hyper. See crosbug.com/21842.
+  EXPECT_EQ(ShiftMask | 0x0U,
+            manager.NormalizeModifiers(XK_Super_L, ShiftMask, true));
+  EXPECT_EQ(Mod1Mask | kReleaseMask,
+            manager.NormalizeModifiers(XK_Super_R, Mod1Mask, false));
+  EXPECT_EQ(ShiftMask | kReleaseMask,
+            manager.NormalizeModifiers(XK_Hyper_L, ShiftMask, false));
+  EXPECT_EQ(Mod1Mask | 0x0U,
+            manager.NormalizeModifiers(XK_Hyper_R, Mod1Mask, true));
 }
 
 TEST(HotkeyManagerTest, TestAddBasic) {