Fix cmd-opt-left/right.

This surfaced because I moved global shortcut handling before menu dispatch recently, and global keyboard handling didn't look at the opt button, so it thought cmd-opt-left meant cmd-left and swallowed the event.

BUG=25396
TEST=Focus text box in web. cmd-opt-left/right should switch tabs, cmd-left/right should move cursor in text box. Focus web background. cmd-opt-left/right should switch tabs, cmd-left/right should do history navigation. Focus omnibox. cmd-opt-left/right should still switch tabs, cmd-left/right should move cursor. cmd-1/2 should switch tabs in all three places, cmd-opt-1/2 should not do anything.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=29646
Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=29648

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

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

4 files changed, 46 insertions, 35 deletions

diff --git a/chrome/browser/cocoa/chrome_event_processing_window.mm b/chrome/browser/cocoa/chrome_event_processing_window.mm
index d8755bf..e5c569a 100644
--- a/chrome/browser/cocoa/chrome_event_processing_window.mm
+++ b/chrome/browser/cocoa/chrome_event_processing_window.mm
@@ -11,7 +11,7 @@
 #import "chrome/browser/renderer_host/render_widget_host_view_mac.h"
 #include "chrome/browser/global_keyboard_shortcuts_mac.h"
 
-typedef int (*KeyToCommandMapper)(bool, bool, bool, int);
+typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int);
 
 @implementation ChromeEventProcessingWindow
 
@@ -22,9 +22,10 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, int);
   const bool cmdKey = modifers & NSCommandKeyMask;
   const bool shiftKey = modifers & NSShiftKeyMask;
   const bool cntrlKey = modifers & NSControlKeyMask;
+  const bool optKey = modifers & NSAlternateKeyMask;
   const int keyCode = [event keyCode];
 
-  int cmdNum = commandForKeyboardShortcut(cmdKey, shiftKey, cntrlKey,
+  int cmdNum = commandForKeyboardShortcut(cmdKey, shiftKey, cntrlKey, optKey,
       keyCode);
 
   BrowserWindowController* controller =
diff --git a/chrome/browser/global_keyboard_shortcuts_mac.h b/chrome/browser/global_keyboard_shortcuts_mac.h
index ca24dd4..13a4587 100644
--- a/chrome/browser/global_keyboard_shortcuts_mac.h
+++ b/chrome/browser/global_keyboard_shortcuts_mac.h
@@ -11,6 +11,7 @@ struct KeyboardShortcutData {
   bool command_key;
   bool shift_key;
   bool cntrl_key;
+  bool opt_key;
   int vkey_code;  // Virtual Key code for the command.
   int chrome_command;  // The chrome command # to execute for this shortcut.
 };
@@ -34,13 +35,15 @@ struct KeyboardShortcutData {
 // opportunity to override the shortcut (with the exception of the tab contents,
 // which first checks if the current web page wants to handle the shortcut).
 int CommandForWindowKeyboardShortcut(
-    bool command_key, bool shift_key, bool cntrl_key, int vkey_code);
+    bool command_key, bool shift_key, bool cntrl_key, bool opt_key,
+    int vkey_code);
 
 // This returns shortcuts that should work only if the tab contents have focus
 // (e.g. cmd-left, which shouldn't do history navigation if e.g. the omnibox has
 // focus).
 int CommandForBrowserKeyboardShortcut(
-    bool command_key, bool shift_key, bool cntrl_key, int vkey_code);
+    bool command_key, bool shift_key, bool cntrl_key, bool opt_key,
+    int vkey_code);
 
 // For testing purposes.
 const KeyboardShortcutData* GetWindowKeyboardShortcutTable(size_t* num_entries);
diff --git a/chrome/browser/global_keyboard_shortcuts_mac.mm b/chrome/browser/global_keyboard_shortcuts_mac.mm
index 3379b5a..1d97f99 100644
--- a/chrome/browser/global_keyboard_shortcuts_mac.mm
+++ b/chrome/browser/global_keyboard_shortcuts_mac.mm
@@ -12,22 +12,22 @@
 const KeyboardShortcutData* GetWindowKeyboardShortcutTable
     (size_t* num_entries) {
   static const KeyboardShortcutData keyboard_shortcuts[] = {
-    {true,  true,  false, kVK_ANSI_RightBracket, IDC_SELECT_NEXT_TAB},
-    {false, false, true,  kVK_PageDown,      IDC_SELECT_NEXT_TAB},
-    {false, false, true,  kVK_Tab,           IDC_SELECT_NEXT_TAB},
-    {true,  true,  false, kVK_ANSI_LeftBracket, IDC_SELECT_PREVIOUS_TAB},
-    {false, false, true,  kVK_PageUp,        IDC_SELECT_PREVIOUS_TAB},
-    {false, true,  true,  kVK_Tab,           IDC_SELECT_PREVIOUS_TAB},
+    {true,  true,  false, false, kVK_ANSI_RightBracket, IDC_SELECT_NEXT_TAB},
+    {false, false, true,  false, kVK_PageDown,      IDC_SELECT_NEXT_TAB},
+    {false, false, true,  false, kVK_Tab,           IDC_SELECT_NEXT_TAB},
+    {true,  true,  false, false, kVK_ANSI_LeftBracket, IDC_SELECT_PREVIOUS_TAB},
+    {false, false, true,  false, kVK_PageUp,        IDC_SELECT_PREVIOUS_TAB},
+    {false, true,  true,  false, kVK_Tab,           IDC_SELECT_PREVIOUS_TAB},
     // Cmd-0..8 select the Nth tab, with cmd-9 being "last tab".
-    {true,  false, false, kVK_ANSI_1,        IDC_SELECT_TAB_0},
-    {true,  false, false, kVK_ANSI_2,        IDC_SELECT_TAB_1},
-    {true,  false, false, kVK_ANSI_3,        IDC_SELECT_TAB_2},
-    {true,  false, false, kVK_ANSI_4,        IDC_SELECT_TAB_3},
-    {true,  false, false, kVK_ANSI_5,        IDC_SELECT_TAB_4},
-    {true,  false, false, kVK_ANSI_6,        IDC_SELECT_TAB_5},
-    {true,  false, false, kVK_ANSI_7,        IDC_SELECT_TAB_6},
-    {true,  false, false, kVK_ANSI_8,        IDC_SELECT_TAB_7},
-    {true,  false, false, kVK_ANSI_9,        IDC_SELECT_LAST_TAB},
+    {true,  false, false, false, kVK_ANSI_1,        IDC_SELECT_TAB_0},
+    {true,  false, false, false, kVK_ANSI_2,        IDC_SELECT_TAB_1},
+    {true,  false, false, false, kVK_ANSI_3,        IDC_SELECT_TAB_2},
+    {true,  false, false, false, kVK_ANSI_4,        IDC_SELECT_TAB_3},
+    {true,  false, false, false, kVK_ANSI_5,        IDC_SELECT_TAB_4},
+    {true,  false, false, false, kVK_ANSI_6,        IDC_SELECT_TAB_5},
+    {true,  false, false, false, kVK_ANSI_7,        IDC_SELECT_TAB_6},
+    {true,  false, false, false, kVK_ANSI_8,        IDC_SELECT_TAB_7},
+    {true,  false, false, false, kVK_ANSI_9,        IDC_SELECT_LAST_TAB},
   };
 
   *num_entries = arraysize(keyboard_shortcuts);
@@ -38,10 +38,10 @@ const KeyboardShortcutData* GetWindowKeyboardShortcutTable
 const KeyboardShortcutData* GetBrowserKeyboardShortcutTable
     (size_t* num_entries) {
   static const KeyboardShortcutData keyboard_shortcuts[] = {
-    {true,  false, false, kVK_LeftArrow,     IDC_BACK},
-    {true,  false, false, kVK_RightArrow,    IDC_FORWARD},
-    {false, false, false, kVK_Delete,        IDC_BACK},
-    {false, true,  false, kVK_Delete,        IDC_FORWARD},
+    {true,  false, false, false, kVK_LeftArrow,     IDC_BACK},
+    {true,  false, false, false, kVK_RightArrow,    IDC_FORWARD},
+    {false, false, false, false, kVK_Delete,        IDC_BACK},
+    {false, true,  false, false, kVK_Delete,        IDC_FORWARD},
   };
 
   *num_entries = arraysize(keyboard_shortcuts);
@@ -51,7 +51,8 @@ const KeyboardShortcutData* GetBrowserKeyboardShortcutTable
 
 static int CommandForKeyboardShortcut(
     const KeyboardShortcutData* (*get_keyboard_shortcut_table)(size_t*),
-    bool command_key, bool shift_key, bool cntrl_key, int vkey_code) {
+    bool command_key, bool shift_key, bool cntrl_key, bool opt_key,
+    int vkey_code) {
 
   // Scan through keycodes and see if it corresponds to one of the global
   // shortcuts on file.
@@ -64,6 +65,7 @@ static int CommandForKeyboardShortcut(
     if (it->command_key == command_key &&
         it->shift_key == shift_key &&
         it->cntrl_key == cntrl_key &&
+        it->opt_key == opt_key &&
         it->vkey_code == vkey_code) {
       return it->chrome_command;
     }
@@ -73,15 +75,17 @@ static int CommandForKeyboardShortcut(
 }
 
 int CommandForWindowKeyboardShortcut(
-    bool command_key, bool shift_key, bool cntrl_key, int vkey_code) {
+    bool command_key, bool shift_key, bool cntrl_key, bool opt_key,
+    int vkey_code) {
   return CommandForKeyboardShortcut(GetWindowKeyboardShortcutTable,
                                     command_key, shift_key,
-                                    cntrl_key, vkey_code);
+                                    cntrl_key, opt_key, vkey_code);
 }
 
 int CommandForBrowserKeyboardShortcut(
-    bool command_key, bool shift_key, bool cntrl_key, int vkey_code) {
+    bool command_key, bool shift_key, bool cntrl_key, bool opt_key,
+    int vkey_code) {
   return CommandForKeyboardShortcut(GetBrowserKeyboardShortcutTable,
                                     command_key, shift_key,
-                                    cntrl_key, vkey_code);
+                                    cntrl_key, opt_key, vkey_code);
 }
diff --git a/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc b/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc
index 8ab7237..39199bb 100644
--- a/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc
+++ b/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc
@@ -10,7 +10,8 @@
 
 TEST(GlobalKeyboardShortcuts, ShortcutsToWindowCommand) {
   // Test that an invalid shortcut translates into an invalid command id.
-  ASSERT_EQ(-1, CommandForWindowKeyboardShortcut(false, false, false, 0));
+  ASSERT_EQ(
+      -1, CommandForWindowKeyboardShortcut(false, false, false, false, 0));
 
   // Check that all known keyboard shortcuts return valid results.
   size_t num_shortcuts = 0;
@@ -19,7 +20,8 @@ TEST(GlobalKeyboardShortcuts, ShortcutsToWindowCommand) {
   ASSERT_GT(num_shortcuts, 0U);
   for (size_t i = 0; i < num_shortcuts; ++i, ++it) {
     int cmd_num = CommandForWindowKeyboardShortcut(
-        it->command_key, it->shift_key, it->cntrl_key, it->vkey_code);
+        it->command_key, it->shift_key, it->cntrl_key, it->opt_key,
+        it->vkey_code);
     ASSERT_EQ(cmd_num, it->chrome_command);
   }
 
@@ -27,14 +29,15 @@ TEST(GlobalKeyboardShortcuts, ShortcutsToWindowCommand) {
   // would be invoked even if e.g. the omnibox had focus, where they really
   // should have text editing functionality).
   ASSERT_EQ(-1, CommandForWindowKeyboardShortcut(
-      true, false, false, kVK_LeftArrow));
+      true, false, false, false, kVK_LeftArrow));
   ASSERT_EQ(-1, CommandForWindowKeyboardShortcut(
-      false, false, false, kVK_Delete));
+      false, false, false, false, kVK_Delete));
 }
 
 TEST(GlobalKeyboardShortcuts, ShortcutsToBrowserCommand) {
   // Test that an invalid shortcut translates into an invalid command id.
-  ASSERT_EQ(-1, CommandForBrowserKeyboardShortcut(false, false, false, 0));
+  ASSERT_EQ(
+      -1, CommandForBrowserKeyboardShortcut(false, false, false, false, 0));
 
   // Check that all known keyboard shortcuts return valid results.
   size_t num_shortcuts = 0;
@@ -43,7 +46,7 @@ TEST(GlobalKeyboardShortcuts, ShortcutsToBrowserCommand) {
   ASSERT_GT(num_shortcuts, 0U);
   for (size_t i = 0; i < num_shortcuts; ++i, ++it) {
     int cmd_num = CommandForBrowserKeyboardShortcut(
-        it->command_key, it->shift_key, it->cntrl_key, it->vkey_code);
-    ASSERT_EQ(cmd_num, it->chrome_command);
+        it->command_key, it->shift_key, it->cntrl_key, it->opt_key,
+        it->vkey_code);    ASSERT_EQ(cmd_num, it->chrome_command);
   }
 }