From 3641da6c02910960ef419d38a161081a9ce1d7b2 Mon Sep 17 00:00:00 2001 From: "jeremy@chromium.org" Date: Wed, 8 Jul 2009 14:59:06 +0000 Subject: Add facitility for Global Keyboard shortcuts. Also, do some housekeeping for our current shortcuts. * Command-Option-l - Downloads [same shortcut as Safari]. * Command-Shift-[/] - Back/forward tab. * cntrl-pageup/pagedn - Back/forward tab. Global Keyboard events are intercepted by a new BrowserWindow custom class. BUG=12537,15486 TEST=Check that above keyboard shortcuts work as advertised. Review URL: http://codereview.chromium.org/149325 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20145 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/nibs/en.lproj/BrowserWindow.xib | 12 +++- chrome/app/nibs/en.lproj/MainMenu.xib | 64 ++++++++++++++++++---- chrome/browser/cocoa/browser_window.h | 20 +++++++ chrome/browser/cocoa/browser_window.mm | 37 +++++++++++++ chrome/browser/cocoa/browser_window_controller.h | 5 ++ chrome/browser/cocoa/browser_window_controller.mm | 7 +++ chrome/browser/global_keyboard_shortcuts_mac.h | 27 +++++++++ chrome/browser/global_keyboard_shortcuts_mac.mm | 43 +++++++++++++++ .../global_keyboard_shortcuts_mac_unittest.cc | 22 ++++++++ chrome/chrome.gyp | 5 ++ 10 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 chrome/browser/cocoa/browser_window.h create mode 100644 chrome/browser/cocoa/browser_window.mm create mode 100644 chrome/browser/global_keyboard_shortcuts_mac.h create mode 100644 chrome/browser/global_keyboard_shortcuts_mac.mm create mode 100644 chrome/browser/global_keyboard_shortcuts_mac_unittest.cc diff --git a/chrome/app/nibs/en.lproj/BrowserWindow.xib b/chrome/app/nibs/en.lproj/BrowserWindow.xib index 306c83d..c622976 100644 --- a/chrome/app/nibs/en.lproj/BrowserWindow.xib +++ b/chrome/app/nibs/en.lproj/BrowserWindow.xib @@ -8,8 +8,8 @@ 353.00 YES - + YES @@ -41,7 +41,7 @@ {{60, 229}, {750, 600}} 536872960 - NSWindow + ChromeBrowserWindow {3.40282e+38, 3.40282e+38} {400, 250} @@ -307,6 +307,14 @@ + ChromeBrowserWindow + NSWindow + + IBProjectSource + browser/cocoa/browser_window.h + + + FirstResponder NSObject diff --git a/chrome/app/nibs/en.lproj/MainMenu.xib b/chrome/app/nibs/en.lproj/MainMenu.xib index 267920a..0e9a336 100644 --- a/chrome/app/nibs/en.lproj/MainMenu.xib +++ b/chrome/app/nibs/en.lproj/MainMenu.xib @@ -8,7 +8,7 @@ 353.00 YES - + YES @@ -787,6 +787,26 @@ 38003 + + + YES + YES + + + 2147483647 + + + + + + Downloads + l + 1572864 + 2147483647 + + + 40012 + YES @@ -1603,6 +1623,14 @@ + commandDispatch: + + + + 633 + + + orderFrontStandardAboutPanel: @@ -2098,6 +2126,8 @@ + + @@ -2453,6 +2483,16 @@ + + 631 + + + + + 632 + + + @@ -2659,6 +2699,8 @@ 58.IBPluginDependency 58.ImportedFromIB2 625.IBPluginDependency + 631.IBPluginDependency + 632.IBPluginDependency 72.IBPluginDependency 72.ImportedFromIB2 73.IBPluginDependency @@ -2751,7 +2793,7 @@ com.apple.InterfaceBuilder.CocoaPlugin - {{647, 825}, {243, 263}} + {{678, 873}, {243, 263}} com.apple.InterfaceBuilder.CocoaPlugin {{197, 734}, {243, 243}} @@ -2797,17 +2839,17 @@ com.apple.InterfaceBuilder.CocoaPlugin - {{933, 1202}, {213, 163}} + {{933, 973}, {213, 163}} com.apple.InterfaceBuilder.CocoaPlugin {{525, 802}, {197, 73}} - {{530, 1365}, {535, 20}} + {{530, 1136}, {535, 20}} com.apple.InterfaceBuilder.CocoaPlugin {74, 862} {{11, 977}, {478, 20}} com.apple.InterfaceBuilder.CocoaPlugin - {{722, 1152}, {287, 213}} + {{722, 893}, {301, 243}} com.apple.InterfaceBuilder.CocoaPlugin {{475, 832}, {234, 43}} com.apple.InterfaceBuilder.CocoaPlugin @@ -2840,7 +2882,7 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{920, 545}, {64, 6}} + {{1023, 930}, {64, 6}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -2850,7 +2892,7 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{764, 688}, {254, 33}} + {{839, 1103}, {254, 33}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -2861,7 +2903,7 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{781, 941}, {188, 123}} + {{772, 1013}, {188, 123}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -2887,6 +2929,8 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + com.apple.InterfaceBuilder.CocoaPlugin + com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -2900,7 +2944,7 @@ com.apple.InterfaceBuilder.CocoaPlugin - {{605, 765}, {249, 323}} + {{636, 813}, {249, 323}} com.apple.InterfaceBuilder.CocoaPlugin {{323, 672}, {199, 203}} @@ -2932,7 +2976,7 @@ - 630 + 633 diff --git a/chrome/browser/cocoa/browser_window.h b/chrome/browser/cocoa/browser_window.h new file mode 100644 index 0000000..3c43577 --- /dev/null +++ b/chrome/browser/cocoa/browser_window.h @@ -0,0 +1,20 @@ +// Copyright (c) 2009 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. + +#ifndef CHROME_BROWSER_COCOA_BROWSER_WINDOW_H_ +#define CHROME_BROWSER_COCOA_BROWSER_WINDOW_H_ + +#import + +// Cocoa class representing a Chrome browser window. +// We need to override NSWindow with our own class since we need access to all +// unhandled keyboard events and subclassing NSWindow is the only method to do +// this. +@interface ChromeBrowserWindow : NSWindow + +// Override, so we can handle global keyboard events. +- (BOOL)performKeyEquivalent:(NSEvent*)theEvent; +@end + +#endif // CHROME_BROWSER_COCOA_BROWSER_WINDOW_H_ diff --git a/chrome/browser/cocoa/browser_window.mm b/chrome/browser/cocoa/browser_window.mm new file mode 100644 index 0000000..0268dc6 --- /dev/null +++ b/chrome/browser/cocoa/browser_window.mm @@ -0,0 +1,37 @@ +// Copyright (c) 2009 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. + +#import "chrome/browser/cocoa/browser_window.h" + +#import "chrome/browser/cocoa/browser_window_controller.h" +#include "chrome/browser/global_keyboard_shortcuts_mac.h" + +@implementation ChromeBrowserWindow + +- (BOOL)performKeyEquivalent:(NSEvent*)event { + // Extract info from |event|. + NSUInteger modifers = [event modifierFlags]; + const bool cmdKey = modifers & NSCommandKeyMask; + const bool shiftKey = modifers & NSShiftKeyMask; + const bool cntrlKey = modifers & NSControlKeyMask; + const int keyCode = [event keyCode]; + + int cmdNum = CommandForKeyboardShortcut(cmdKey, shiftKey, cntrlKey, + keyCode); + + BrowserWindowController* controller = + (BrowserWindowController*)[self delegate]; + // A bit of sanity. + DCHECK([controller isKindOfClass:[BrowserWindowController class]]); + DCHECK([controller respondsToSelector:@selector(executeCommand:)]); + + if (cmdNum != -1) { + [controller executeCommand:cmdNum]; + return YES; + } + + return [super performKeyEquivalent:event]; +} + +@end diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h index c7b24f1..54ed849 100644 --- a/chrome/browser/cocoa/browser_window_controller.h +++ b/chrome/browser/cocoa/browser_window_controller.h @@ -117,6 +117,11 @@ class TabStripModelObserverBridge; // Returns fullscreen state. - (BOOL)isFullscreen; +// Executes the command in the context of the current browser. +// |command| is an integer value containing one of the constants defined in the +// "chrome/app/chrome_dll_resource.h" file. +- (void)executeCommand:(int)command; + @end diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm index da2ede6..041d6e1 100644 --- a/chrome/browser/cocoa/browser_window_controller.mm +++ b/chrome/browser/cocoa/browser_window_controller.mm @@ -375,6 +375,13 @@ willPositionSheet:(NSWindow *)sheet browser_->ExecuteCommand(tag); } +// Called when another part of the internal codebase needs to execute a +// command. +- (void)executeCommand:(int)command { + if (browser_->command_updater()->IsCommandEnabled(command)) + browser_->ExecuteCommand(command); +} + - (LocationBar*)locationBar { return [toolbarController_ locationBar]; } diff --git a/chrome/browser/global_keyboard_shortcuts_mac.h b/chrome/browser/global_keyboard_shortcuts_mac.h new file mode 100644 index 0000000..45d9a69 --- /dev/null +++ b/chrome/browser/global_keyboard_shortcuts_mac.h @@ -0,0 +1,27 @@ +// Copyright (c) 2009 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. + +#ifndef CHROME_BROWSER_GLOBAL_KEYBOARD_SHORTCUTS_MAC_H_ +#define CHROME_BROWSER_GLOBAL_KEYBOARD_SHORTCUTS_MAC_H_ + +#include "base/basictypes.h"; + +struct KeyboardShortcutData { + bool command_key; + bool shift_key; + bool cntrl_key; + int vkey_code; // Virtual Key code for the command. + int chrome_command; // The chrome command # to execute for this shortcut. +}; + +// Check if a given keycode + modifiers correspond to a given Chrome command. +// returns: Command number (as passed to Browser::ExecuteCommand) or -1 if there +// was no match. +int CommandForKeyboardShortcut(bool command_key, bool shift_key, bool cntrl_key, + int vkey_code); + +// For testing purposes. +const KeyboardShortcutData* GetKeyboardShortCutTable(size_t* num_entries); + +#endif // #ifndef CHROME_BROWSER_GLOBAL_KEYBOARD_SHORTCUTS_MAC_H_ diff --git a/chrome/browser/global_keyboard_shortcuts_mac.mm b/chrome/browser/global_keyboard_shortcuts_mac.mm new file mode 100644 index 0000000..426d379 --- /dev/null +++ b/chrome/browser/global_keyboard_shortcuts_mac.mm @@ -0,0 +1,43 @@ +// Copyright (c) 2009 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/global_keyboard_shortcuts_mac.h" + +#include "base/basictypes.h" +#include "chrome/app/chrome_dll_resource.h" + +const KeyboardShortcutData* GetKeyboardShortCutTable(size_t* num_entries) { + static const KeyboardShortcutData keyboard_shortcuts[] = { + {true, true, false, 30 /* ] */, IDC_SELECT_NEXT_TAB}, + {false, false, true, 121 /* pg down */, IDC_SELECT_NEXT_TAB}, + {true, true, false, 33 /* [ */, IDC_SELECT_PREVIOUS_TAB}, + {false, false, true, 116 /* pg_up */, IDC_SELECT_PREVIOUS_TAB}, + }; + + *num_entries = arraysize(keyboard_shortcuts); + + return keyboard_shortcuts; +} + +int CommandForKeyboardShortcut(bool command_key, bool shift_key, bool cntrl_key, + int vkey_code) { + + // Scan through keycodes and see if it corresponds to one of the global + // shortcuts on file. + // + // TODO(jeremy): Change this into a hash table once we get enough + // entries in the array to make a difference. + size_t num_shortcuts = 0; + const KeyboardShortcutData *it = GetKeyboardShortCutTable(&num_shortcuts); + for (size_t i = 0; i < num_shortcuts; ++i, ++it) { + if (it->command_key == command_key && + it->shift_key == shift_key && + it->cntrl_key == cntrl_key && + it->vkey_code == vkey_code) { + return it->chrome_command; + } + } + + return -1; +} diff --git a/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc b/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc new file mode 100644 index 0000000..c8c22d2 --- /dev/null +++ b/chrome/browser/global_keyboard_shortcuts_mac_unittest.cc @@ -0,0 +1,22 @@ +// Copyright (c) 2009 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/global_keyboard_shortcuts_mac.h" + +#include "testing/gtest/include/gtest/gtest.h" + +TEST(GlobalKeyboardShortcuts, ShortCutsToCommand) { + // Test that an invalid shortcut translates into an invalid command id. + ASSERT_EQ(CommandForKeyboardShortcut(false, false, false, 0), -1); + + // Check that all known keyboard shortcuts return valid results. + size_t num_shortcuts = 0; + const KeyboardShortcutData *it = GetKeyboardShortCutTable(&num_shortcuts); + ASSERT_GT(num_shortcuts, 0U); + for (size_t i = 0; i < num_shortcuts; ++i, ++it) { + int cmd_num = CommandForKeyboardShortcut(it->command_key, it->shift_key, + it->cntrl_key, it->vkey_code); + ASSERT_EQ(cmd_num, it->chrome_command); + } +} diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index 8dca557..d5a1733 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -732,6 +732,8 @@ 'browser/cocoa/bookmark_menu_cocoa_controller.h', 'browser/cocoa/bookmark_menu_cocoa_controller.mm', 'browser/cocoa/browser_test_helper.h', + 'browser/cocoa/browser_window.h', + 'browser/cocoa/browser_window.mm', 'browser/cocoa/browser_window_cocoa.h', 'browser/cocoa/browser_window_cocoa.mm', 'browser/cocoa/browser_window_controller.h', @@ -934,6 +936,8 @@ 'browser/external_protocol_handler.h', 'browser/external_tab_container.cc', 'browser/external_tab_container.h', + 'browser/global_keyboard_shortcuts_mac.h', + 'browser/global_keyboard_shortcuts_mac.mm', 'browser/fav_icon_helper.cc', 'browser/fav_icon_helper.h', 'browser/find_bar.h', @@ -3486,6 +3490,7 @@ 'browser/extensions/extensions_service_unittest.cc', 'browser/extensions/user_script_master_unittest.cc', 'browser/find_backend_unittest.cc', + 'browser/global_keyboard_shortcuts_mac_unittest.cc', 'browser/google_url_tracker_unittest.cc', 'browser/google_update_settings_linux_unittest.cc', 'browser/google_update_settings_mac_unittest.mm', -- cgit v1.1