Enable Search-key modifiers for extended key shortcuts.

chrome/browser/ui/ash/event_rewriter.cc

Index: chrome/browser/ui/ash/event_rewriter.cc
diff --git a/chrome/browser/ui/ash/event_rewriter.cc b/chrome/browser/ui/ash/event_rewriter.cc
index 74d71d914348ced6dbf56a0f7058445ce8b44fdf..27eced789c11205d552b2e365b797d1a1dd131fe 100644
--- a/chrome/browser/ui/ash/event_rewriter.cc
+++ b/chrome/browser/ui/ash/event_rewriter.cc
@@ -634,9 +634,9 @@ bool EventRewriter::RewriteNumPadKeys(ui::KeyEvent* event) {
 
 bool EventRewriter::RewriteExtendedKeys(ui::KeyEvent* event) {
 #if defined(OS_CHROMEOS)
-  const bool search_as_function_key =
-      CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableChromebookFunctionKey);
+  const bool keep_non_search_shortcuts =
+      !CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableNonSearchExtendedKeyShortcuts);
 
   XEvent* xev = event->native_event();
   XKeyEvent* xkey = &(xev->xkey);
@@ -647,7 +647,50 @@ bool EventRewriter::RewriteExtendedKeys(ui::KeyEvent* event) {
   ui::KeyboardCode remapped_keycode = ui::VKEY_UNKNOWN;
   unsigned int remapped_mods = 0;
 
-  if (!search_as_function_key) {
+  static const KeyboardRemapping remappings[] = {
+    { // Search+BackSpace -> Delete
+      XK_BackSpace,
+      0, Mod4Mask,
+      XK_Delete, ui::VKEY_DELETE,
+    },
+    { // Search+Left -> Home
+      XK_Left,
+      0, Mod4Mask,
+      XK_Home, ui::VKEY_HOME,
+    },
+    { // Search+Up -> Prior (aka PageUp)
+      XK_Up,
+      0, Mod4Mask,
+      XK_Prior, ui::VKEY_PRIOR,
+    },
+    { // Search+Right -> End
+      XK_Right,
+      0, Mod4Mask,
+      XK_End, ui::VKEY_END,
+    },
+    { // Search+Down -> Next (aka PageDown)
+      XK_Down,
+      0, Mod4Mask,
+      XK_Next, ui::VKEY_NEXT,
+    },
+    { // Search+Period -> Insert
+      XK_period,
+      0, Mod4Mask,
+      XK_Insert, ui::VKEY_INSERT,
+    }
+  };
+
+  RewriteWithKeyboardRemappingsByKeySym(remappings,
+                                        arraysize(remappings),
+                                        keysym,
+                                        xkey->state,
+                                        event->flags(),
+                                        &remapped_native_keysym,
+                                        &remapped_native_mods,
+                                        &remapped_keycode,
+                                        &remapped_mods);
+
+  if (keep_non_search_shortcuts && remapped_keycode == ui::VKEY_UNKNOWN) {

[Yusuke Sato, 2012/12/17 00:26:17]

Do we really need the new chrome://flag? Our UX team explicitly told us that
it's not possible to remove the old shortcuts such as Alt+BS.

Since the old one (Alt+BS etc) and the new one (Search+BS etc) can coexist
without any problem, I'd prefer to change the line to 'if (remapped_keycode ==
ui::VKEY_UNKNOWN)' and remove the new flag.

[danakj, 2012/12/17 02:05:14]

Ok. The one downside is you have to choose between Search+Alt+Backspace as
becoming Alt+Backspace or Alt+Delete, but you can't access both keys. Or same
for Ctrl+Alt+PageDown. If that becomes an issue we can revisit it. I've removed
the flag entirely for now.

     static const KeyboardRemapping remappings[] = {
       { // Alt+BackSpace -> Delete
         XK_BackSpace,
@@ -685,52 +728,9 @@ bool EventRewriter::RewriteExtendedKeys(ui::KeyEvent* event) {
                                           &remapped_native_mods,
                                           &remapped_keycode,
                                           &remapped_mods);
-  } else {
-    static const KeyboardRemapping remappings[] = {
-      { // Search+BackSpace -> Delete
-        XK_BackSpace,
-        0, Mod4Mask,
-        XK_Delete, ui::VKEY_DELETE,
-      },
-      { // Search+Left -> Home
-        XK_Left,
-        0, Mod4Mask,
-        XK_Home, ui::VKEY_HOME,
-      },
-      { // Search+Up -> Prior (aka PageUp)
-        XK_Up,
-        0, Mod4Mask,
-        XK_Prior, ui::VKEY_PRIOR,
-      },
-      { // Search+Right -> End
-        XK_Right,
-        0, Mod4Mask,
-        XK_End, ui::VKEY_END,
-      },
-      { // Search+Down -> Next (aka PageDown)
-        XK_Down,
-        0, Mod4Mask,
-        XK_Next, ui::VKEY_NEXT,
-      },
-      { // Search+Period -> Insert
-        XK_period,
-        0, Mod4Mask,
-        XK_Insert, ui::VKEY_INSERT,
-      }
-    };
-
-    RewriteWithKeyboardRemappingsByKeySym(remappings,
-                                          arraysize(remappings),
-                                          keysym,
-                                          xkey->state,
-                                          event->flags(),
-                                          &remapped_native_keysym,
-                                          &remapped_native_mods,
-                                          &remapped_keycode,
-                                          &remapped_mods);
   }
 
-  if (!remapped_native_keysym || remapped_keycode == ui::VKEY_UNKNOWN)
+  if (remapped_keycode == ui::VKEY_UNKNOWN)
     return false;
 
   OverwriteEvent(event,
@@ -747,10 +747,6 @@ bool EventRewriter::RewriteExtendedKeys(ui::KeyEvent* event) {
 
 bool EventRewriter::RewriteFunctionKeys(ui::KeyEvent* event) {
 #if defined(OS_CHROMEOS)
-  const bool search_as_function_key =
-      CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableChromebookFunctionKey);
-
   XEvent* xev = event->native_event();
   XKeyEvent* xkey = &(xev->xkey);
   const KeySym keysym = XLookupKeysym(xkey, 0);
@@ -760,9 +756,7 @@ bool EventRewriter::RewriteFunctionKeys(ui::KeyEvent* event) {
   ui::KeyboardCode remapped_keycode = ui::VKEY_UNKNOWN;
   unsigned int remapped_mods = 0;
 
-  bool remapped = false;
-
-  if (search_as_function_key && xkey->state & Mod4Mask) {
+  if (xkey->state & Mod4Mask) {
     // Allow Search to avoid rewriting F1-F12.
     static const KeyboardRemapping kFkeysToFkeys[] = {
       { XK_F1, 0, Mod4Mask, XK_F1, ui::VKEY_F1, },
@@ -779,19 +773,18 @@ bool EventRewriter::RewriteFunctionKeys(ui::KeyEvent* event) {
       { XK_F12, 0, Mod4Mask, XK_F12, ui::VKEY_F12, },
     };
 
-    remapped =
-        RewriteWithKeyboardRemappingsByKeySym(kFkeysToFkeys,
-                                              arraysize(kFkeysToFkeys),
-                                              keysym,
-                                              xkey->state,
-                                              event->flags(),
-                                              &remapped_native_keysym,
-                                              &remapped_native_mods,
-                                              &remapped_keycode,
-                                              &remapped_mods);
+    RewriteWithKeyboardRemappingsByKeySym(kFkeysToFkeys,
+                                          arraysize(kFkeysToFkeys),
+                                          keysym,
+                                          xkey->state,
+                                          event->flags(),
+                                          &remapped_native_keysym,
+                                          &remapped_native_mods,
+                                          &remapped_keycode,
+                                          &remapped_mods);
   }
 
-  if (!remapped) {
+  if (remapped_keycode == ui::VKEY_UNKNOWN) {
     // Rewrite the actual F1-F12 keys on a Chromebook keyboard to special keys.
     static const KeyboardRemapping kFkeysToSpecialKeys[] = {
       { XK_F1, 0, 0, XF86XK_Back, ui::VKEY_BROWSER_BACK, },
@@ -806,55 +799,52 @@ bool EventRewriter::RewriteFunctionKeys(ui::KeyEvent* event) {
       { XK_F10, 0, 0, XF86XK_AudioRaiseVolume, ui::VKEY_VOLUME_UP, },
     };
 
-    remapped =
-        RewriteWithKeyboardRemappingsByKeySym(kFkeysToSpecialKeys,
-                                              arraysize(kFkeysToSpecialKeys),
-                                              keysym,
-                                              xkey->state,
-                                              event->flags(),
-                                              &remapped_native_keysym,
-                                              &remapped_native_mods,
-                                              &remapped_keycode,
-                                              &remapped_mods);
+    RewriteWithKeyboardRemappingsByKeySym(kFkeysToSpecialKeys,
+                                          arraysize(kFkeysToSpecialKeys),
+                                          keysym,
+                                          xkey->state,
+                                          event->flags(),
+                                          &remapped_native_keysym,
+                                          &remapped_native_mods,
+                                          &remapped_keycode,
+                                          &remapped_mods);
   }
 
-  if (!remapped) {
-    // When using Search as a Function key, remap Search+<number> to F<number>.
-    if (search_as_function_key && xkey->state & Mod4Mask) {
-      // We check the keycode here instead of the keysym, as these keys have
-      // different keysyms when modifiers are pressed, such as shift.
-
-      // TODO(danakj): On some i18n keyboards, these choices will be bad and we
-      // should make layout-specific choices here. For eg. on a french keyboard
-      // "-" and "6" are the same key, so F11 will not be accessible.
-      static const KeyboardRemapping kNumberKeysToFkeys[] = {
-        { XK_1, 0, Mod4Mask, XK_F1, ui::VKEY_F1, },
-        { XK_2, 0, Mod4Mask, XK_F2, ui::VKEY_F2, },
-        { XK_3, 0, Mod4Mask, XK_F3, ui::VKEY_F3, },
-        { XK_4, 0, Mod4Mask, XK_F4, ui::VKEY_F4, },
-        { XK_5, 0, Mod4Mask, XK_F5, ui::VKEY_F5, },
-        { XK_6, 0, Mod4Mask, XK_F6, ui::VKEY_F6, },
-        { XK_7, 0, Mod4Mask, XK_F7, ui::VKEY_F7, },
-        { XK_8, 0, Mod4Mask, XK_F8, ui::VKEY_F8, },
-        { XK_9, 0, Mod4Mask, XK_F9, ui::VKEY_F9, },
-        { XK_0, 0, Mod4Mask, XK_F10, ui::VKEY_F10, },
-        { XK_minus, 0, Mod4Mask, XK_F11, ui::VKEY_F11, },
-        { XK_equal, 0, Mod4Mask, XK_F12, ui::VKEY_F12, }
-      };
-
-      RewriteWithKeyboardRemappingsByKeyCode(kNumberKeysToFkeys,
-                                             arraysize(kNumberKeysToFkeys),
-                                             xkey->keycode,
-                                             xkey->state,
-                                             event->flags(),
-                                             &remapped_native_keysym,
-                                             &remapped_native_mods,
-                                             &remapped_keycode,
-                                             &remapped_mods);
-    }
+  if (remapped_keycode == ui::VKEY_UNKNOWN && xkey->state & Mod4Mask) {
+    // Remap Search+<number> to F<number>.
+    // We check the keycode here instead of the keysym, as these keys have
+    // different keysyms when modifiers are pressed, such as shift.
+
+    // TODO(danakj): On some i18n keyboards, these choices will be bad and we
+    // should make layout-specific choices here. For eg. on a french keyboard
+    // "-" and "6" are the same key, so F11 will not be accessible.
+    static const KeyboardRemapping kNumberKeysToFkeys[] = {
+      { XK_1, 0, Mod4Mask, XK_F1, ui::VKEY_F1, },
+      { XK_2, 0, Mod4Mask, XK_F2, ui::VKEY_F2, },
+      { XK_3, 0, Mod4Mask, XK_F3, ui::VKEY_F3, },
+      { XK_4, 0, Mod4Mask, XK_F4, ui::VKEY_F4, },
+      { XK_5, 0, Mod4Mask, XK_F5, ui::VKEY_F5, },
+      { XK_6, 0, Mod4Mask, XK_F6, ui::VKEY_F6, },
+      { XK_7, 0, Mod4Mask, XK_F7, ui::VKEY_F7, },
+      { XK_8, 0, Mod4Mask, XK_F8, ui::VKEY_F8, },
+      { XK_9, 0, Mod4Mask, XK_F9, ui::VKEY_F9, },
+      { XK_0, 0, Mod4Mask, XK_F10, ui::VKEY_F10, },
+      { XK_minus, 0, Mod4Mask, XK_F11, ui::VKEY_F11, },
+      { XK_equal, 0, Mod4Mask, XK_F12, ui::VKEY_F12, }
+    };
+
+    RewriteWithKeyboardRemappingsByKeyCode(kNumberKeysToFkeys,
+                                           arraysize(kNumberKeysToFkeys),
+                                           xkey->keycode,
+                                           xkey->state,
+                                           event->flags(),
+                                           &remapped_native_keysym,
+                                           &remapped_native_mods,
+                                           &remapped_keycode,
+                                           &remapped_mods);
   }
 
-  if (!remapped_native_keysym || remapped_keycode == ui::VKEY_UNKNOWN)
+  if (remapped_keycode == ui::VKEY_UNKNOWN)
     return false;
 
   OverwriteEvent(event,