Enable Search-key modifiers for extended key shortcuts.

chrome/browser/ui/ash/event_rewriter.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/event_rewriter.h"
 
 #include <vector>
 
 #include "ash/shell.h"
 #include "base/logging.h"
@@ skipping 616 matching lines @@
       break;
   }
 #else
   // TODO(yusukes): Support Ash on other platforms if needed.
 #endif
   return rewritten;
 }
 
 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);
   const KeySym keysym = XLookupKeysym(xkey, 0);
 
   KeySym remapped_native_keysym = 0;
   unsigned int remapped_native_mods = 0;
   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,
         ui::EF_ALT_DOWN, Mod1Mask,
         XK_Delete, ui::VKEY_DELETE,
       },
       { // Control+Alt+Up -> Home
         XK_Up,
         ui::EF_ALT_DOWN | ui::EF_CONTROL_DOWN, Mod1Mask | ControlMask,
         XK_Home, ui::VKEY_HOME,
@@ skipping 17 matching lines @@
 
     RewriteWithKeyboardRemappingsByKeySym(remappings,
                                           arraysize(remappings),
                                           keysym,
                                           xkey->state,
                                           event->flags(),
                                           &remapped_native_keysym,
                                           &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,
                  NativeKeySymToNativeKeycode(remapped_native_keysym),
                  remapped_native_mods,
                  remapped_keycode,
                  remapped_mods);
   return true;
 #else
   // TODO(yusukes): Support Ash on other platforms if needed.
   return false;
 #endif
 }
 
 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);
 
   KeySym remapped_native_keysym = 0;
   unsigned int remapped_native_mods = 0;
   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, },
       { XK_F2, 0, Mod4Mask, XK_F2, ui::VKEY_F2, },
       { XK_F3, 0, Mod4Mask, XK_F3, ui::VKEY_F3, },
       { XK_F4, 0, Mod4Mask, XK_F4, ui::VKEY_F4, },
       { XK_F5, 0, Mod4Mask, XK_F5, ui::VKEY_F5, },
       { XK_F6, 0, Mod4Mask, XK_F6, ui::VKEY_F6, },
       { XK_F7, 0, Mod4Mask, XK_F7, ui::VKEY_F7, },
       { XK_F8, 0, Mod4Mask, XK_F8, ui::VKEY_F8, },
       { XK_F9, 0, Mod4Mask, XK_F9, ui::VKEY_F9, },
       { XK_F10, 0, Mod4Mask, XK_F10, ui::VKEY_F10, },
       { XK_F11, 0, Mod4Mask, XK_F11, ui::VKEY_F11, },
       { 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, },
       { XK_F2, 0, 0, XF86XK_Forward, ui::VKEY_BROWSER_FORWARD, },
       { XK_F3, 0, 0, XF86XK_Reload, ui::VKEY_BROWSER_REFRESH, },
       { XK_F4, 0, 0, XF86XK_LaunchB, ui::VKEY_MEDIA_LAUNCH_APP2, },
       { XK_F5, 0, 0, XF86XK_LaunchA, ui::VKEY_MEDIA_LAUNCH_APP1, },
       { XK_F6, 0, 0, XF86XK_MonBrightnessDown, ui::VKEY_BRIGHTNESS_DOWN, },
       { XK_F7, 0, 0, XF86XK_MonBrightnessUp, ui::VKEY_BRIGHTNESS_UP, },
       { XK_F8, 0, 0, XF86XK_AudioMute, ui::VKEY_VOLUME_MUTE, },
       { XK_F9, 0, 0, XF86XK_AudioLowerVolume, ui::VKEY_VOLUME_DOWN, },
       { 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.
+  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, }
-      };
+    // 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);
-    }
+    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,
                  NativeKeySymToNativeKeycode(remapped_native_keysym),
                  remapped_native_mods,
                  remapped_keycode,
                  remapped_mods);
   return true;
 #else
   // TODO(danakj): Support Ash on other platforms if needed.
@@ skipping 69 matching lines @@
   const DeviceType type = EventRewriter::GetDeviceType(device_name);
   if (type == kDeviceAppleKeyboard) {
     VLOG(1) << "Apple keyboard '" << device_name << "' connected: "
             << "id=" << device_id;
   }
   // Always overwrite the existing device_id since the X server may reuse a
   // device id for an unattached device.
   device_id_to_type_[device_id] = type;
   return type;
 }