Commit c93c3dca authored by erikchen's avatar erikchen Committed by Commit Bot

macOS: Display [ctr + tab] and [ctr + shift + tab] as hotkeys for tab switching.

This matches other macOS application like Safari, Terminal, etc. All existing
hotkeys will still work.

This CL uses the same hotkey for "previous tab" that Safari and Terminal use,
which is [ctr + shift + "Horizontal Tab"]. This also causes tests to pass.
However, pressing that key combination actually generates [ctr + shift + "End of
Medium"], which renderers in the Main Menu as a backwards tab. This CL updates
NSMenuItem(ChromeAdditions) to check for this special case.

This CL removes two incorrect DCHECKs from render_widget_host_view_cocoa.mm. I
regularly hit both of them when running Chromium on a local build. They both
rely on the false assumption that keyEquivalents in the main menu must have the
command modifier.

Bug: 851714
Change-Id: I90c643870f9bc7dd87369496eec9ab62a1685e07
Reviewed-on: https://chromium-review.googlesource.com/1106659
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574211}
parent a9ae38f0
...@@ -481,14 +481,20 @@ CA ...@@ -481,14 +481,20 @@ CA
</connections> </connections>
</menuItem> </menuItem>
<menuItem isSeparatorItem="YES" id="453"/> <menuItem isSeparatorItem="YES" id="453"/>
<menuItem title="^IDS_NEXT_TAB_MAC" tag="34016" keyEquivalent="" id="455"> <menuItem title="^IDS_NEXT_TAB_MAC" tag="34016" id="455">
<modifierMask key="keyEquivalentModifierMask" option="YES" command="YES"/> <string key="keyEquivalent" base64-UTF8="YES">
CQ
</string>
<modifierMask key="keyEquivalentModifierMask" control="YES"/>
<connections> <connections>
<action selector="commandDispatch:" target="-1" id="529"/> <action selector="commandDispatch:" target="-1" id="529"/>
</connections> </connections>
</menuItem> </menuItem>
<menuItem title="^IDS_PREV_TAB_MAC" tag="34017" keyEquivalent="" id="454"> <menuItem title="^IDS_PREV_TAB_MAC" tag="34017" id="454">
<modifierMask key="keyEquivalentModifierMask" option="YES" command="YES"/> <string key="keyEquivalent" base64-UTF8="YES">
CQ
</string>
<modifierMask key="keyEquivalentModifierMask" shift="YES" control="YES"/>
<connections> <connections>
<action selector="commandDispatch:" target="-1" id="530"/> <action selector="commandDispatch:" target="-1" id="530"/>
</connections> </connections>
......
...@@ -123,9 +123,9 @@ const std::vector<KeyboardShortcutData>& GetShortcutsNotPresentInMainMenu() { ...@@ -123,9 +123,9 @@ const std::vector<KeyboardShortcutData>& GetShortcutsNotPresentInMainMenu() {
{true, true, false, false, kVK_ANSI_RightBracket, IDC_SELECT_NEXT_TAB}, {true, true, false, false, kVK_ANSI_RightBracket, IDC_SELECT_NEXT_TAB},
{true, true, false, false, kVK_ANSI_LeftBracket, IDC_SELECT_PREVIOUS_TAB}, {true, true, false, false, kVK_ANSI_LeftBracket, IDC_SELECT_PREVIOUS_TAB},
{false, false, true, false, kVK_PageDown, IDC_SELECT_NEXT_TAB}, {false, false, true, false, kVK_PageDown, IDC_SELECT_NEXT_TAB},
{false, false, true, false, kVK_Tab, IDC_SELECT_NEXT_TAB},
{false, false, true, false, kVK_PageUp, IDC_SELECT_PREVIOUS_TAB}, {false, false, true, false, kVK_PageUp, IDC_SELECT_PREVIOUS_TAB},
{false, true, true, false, kVK_Tab, IDC_SELECT_PREVIOUS_TAB}, {true, false, false, true, kVK_RightArrow, IDC_SELECT_NEXT_TAB},
{true, false, false, true, kVK_LeftArrow, IDC_SELECT_PREVIOUS_TAB},
// Cmd-0..8 select the nth tab, with cmd-9 being "last tab". // Cmd-0..8 select the nth tab, with cmd-9 being "last tab".
{true, false, false, false, kVK_ANSI_1, IDC_SELECT_TAB_0}, {true, false, false, false, kVK_ANSI_1, IDC_SELECT_TAB_0},
......
...@@ -42,67 +42,66 @@ const struct AcceleratorMapping { ...@@ -42,67 +42,66 @@ const struct AcceleratorMapping {
NSUInteger modifiers; // The Cocoa modifiers. NSUInteger modifiers; // The Cocoa modifiers.
ui::KeyboardCode key_code; // The key used for cross-platform compatibility. ui::KeyboardCode key_code; // The key used for cross-platform compatibility.
} kAcceleratorMap[] = { } kAcceleratorMap[] = {
// Accelerators used in the toolbar menu. // Accelerators used in the toolbar menu.
{IDC_CLEAR_BROWSING_DATA, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_BACK}, {IDC_CLEAR_BROWSING_DATA, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_BACK},
{IDC_COPY, NSCommandKeyMask, ui::VKEY_C}, {IDC_COPY, NSCommandKeyMask, ui::VKEY_C},
{IDC_CUT, NSCommandKeyMask, ui::VKEY_X}, {IDC_CUT, NSCommandKeyMask, ui::VKEY_X},
{IDC_DEV_TOOLS, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_I}, {IDC_DEV_TOOLS, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_I},
{IDC_DEV_TOOLS_CONSOLE, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_J}, {IDC_DEV_TOOLS_CONSOLE, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_J},
{IDC_FIND, NSCommandKeyMask, ui::VKEY_F}, {IDC_FIND, NSCommandKeyMask, ui::VKEY_F},
{IDC_FULLSCREEN, NSCommandKeyMask | NSControlKeyMask, ui::VKEY_F}, {IDC_FULLSCREEN, NSCommandKeyMask | NSControlKeyMask, ui::VKEY_F},
{IDC_NEW_INCOGNITO_WINDOW, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_N}, {IDC_NEW_INCOGNITO_WINDOW, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_N},
{IDC_NEW_TAB, NSCommandKeyMask, ui::VKEY_T}, {IDC_NEW_TAB, NSCommandKeyMask, ui::VKEY_T},
{IDC_NEW_WINDOW, NSCommandKeyMask, ui::VKEY_N}, {IDC_NEW_WINDOW, NSCommandKeyMask, ui::VKEY_N},
{IDC_PASTE, NSCommandKeyMask, ui::VKEY_V}, {IDC_PASTE, NSCommandKeyMask, ui::VKEY_V},
{IDC_PRINT, NSCommandKeyMask, ui::VKEY_P}, {IDC_PRINT, NSCommandKeyMask, ui::VKEY_P},
{IDC_RESTORE_TAB, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_T}, {IDC_RESTORE_TAB, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_T},
{IDC_SAVE_PAGE, NSCommandKeyMask, ui::VKEY_S}, {IDC_SAVE_PAGE, NSCommandKeyMask, ui::VKEY_S},
{IDC_SHOW_BOOKMARK_BAR, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_B}, {IDC_SHOW_BOOKMARK_BAR, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_B},
{IDC_SHOW_BOOKMARK_MANAGER, NSCommandKeyMask | NSAlternateKeyMask, {IDC_SHOW_BOOKMARK_MANAGER, NSCommandKeyMask | NSAlternateKeyMask,
ui::VKEY_B}, ui::VKEY_B},
{IDC_BOOKMARK_PAGE, NSCommandKeyMask, ui::VKEY_D}, {IDC_BOOKMARK_PAGE, NSCommandKeyMask, ui::VKEY_D},
{IDC_SHOW_DOWNLOADS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_J}, {IDC_SHOW_DOWNLOADS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_J},
{IDC_SHOW_HISTORY, NSCommandKeyMask, ui::VKEY_Y}, {IDC_SHOW_HISTORY, NSCommandKeyMask, ui::VKEY_Y},
{IDC_VIEW_SOURCE, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_U}, {IDC_VIEW_SOURCE, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_U},
{IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS}, {IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS},
{IDC_ZOOM_PLUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_PLUS}, {IDC_ZOOM_PLUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_PLUS},
// Accelerators used in MainMenu.xib, but not the toolbar menu. // Accelerators used in MainMenu.xib, but not the toolbar menu.
{IDC_HIDE_APP, NSCommandKeyMask, ui::VKEY_H}, {IDC_HIDE_APP, NSCommandKeyMask, ui::VKEY_H},
{IDC_EXIT, NSCommandKeyMask, ui::VKEY_Q}, {IDC_EXIT, NSCommandKeyMask, ui::VKEY_Q},
{IDC_OPEN_FILE, NSCommandKeyMask, ui::VKEY_O}, {IDC_OPEN_FILE, NSCommandKeyMask, ui::VKEY_O},
{IDC_FOCUS_LOCATION, NSCommandKeyMask, ui::VKEY_L}, {IDC_FOCUS_LOCATION, NSCommandKeyMask, ui::VKEY_L},
{IDC_CLOSE_WINDOW, NSCommandKeyMask, ui::VKEY_W}, {IDC_CLOSE_WINDOW, NSCommandKeyMask, ui::VKEY_W},
{IDC_EMAIL_PAGE_LOCATION, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_I}, {IDC_EMAIL_PAGE_LOCATION, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_I},
#if BUILDFLAG(ENABLE_PRINTING) #if BUILDFLAG(ENABLE_PRINTING)
{IDC_BASIC_PRINT, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_P}, {IDC_BASIC_PRINT, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_P},
#endif // ENABLE_PRINTING #endif // ENABLE_PRINTING
{IDC_CONTENT_CONTEXT_UNDO, NSCommandKeyMask, ui::VKEY_Z}, {IDC_CONTENT_CONTEXT_UNDO, NSCommandKeyMask, ui::VKEY_Z},
{IDC_CONTENT_CONTEXT_REDO, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_Z}, {IDC_CONTENT_CONTEXT_REDO, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_Z},
{IDC_CONTENT_CONTEXT_CUT, NSCommandKeyMask, ui::VKEY_X}, {IDC_CONTENT_CONTEXT_CUT, NSCommandKeyMask, ui::VKEY_X},
{IDC_CONTENT_CONTEXT_COPY, NSCommandKeyMask, ui::VKEY_C}, {IDC_CONTENT_CONTEXT_COPY, NSCommandKeyMask, ui::VKEY_C},
{IDC_CONTENT_CONTEXT_PASTE, NSCommandKeyMask, ui::VKEY_V}, {IDC_CONTENT_CONTEXT_PASTE, NSCommandKeyMask, ui::VKEY_V},
{IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE, {IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE,
NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_V}, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_V},
{IDC_CONTENT_CONTEXT_SELECTALL, NSCommandKeyMask, ui::VKEY_A}, {IDC_CONTENT_CONTEXT_SELECTALL, NSCommandKeyMask, ui::VKEY_A},
{IDC_FOCUS_SEARCH, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_F}, {IDC_FOCUS_SEARCH, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_F},
{IDC_FIND_NEXT, NSCommandKeyMask, ui::VKEY_G}, {IDC_FIND_NEXT, NSCommandKeyMask, ui::VKEY_G},
{IDC_FIND_PREVIOUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_G}, {IDC_FIND_PREVIOUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_G},
{IDC_ZOOM_PLUS, NSCommandKeyMask, ui::VKEY_OEM_PLUS}, {IDC_ZOOM_PLUS, NSCommandKeyMask, ui::VKEY_OEM_PLUS},
{IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS}, {IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS},
{IDC_STOP, NSCommandKeyMask, ui::VKEY_OEM_PERIOD}, {IDC_STOP, NSCommandKeyMask, ui::VKEY_OEM_PERIOD},
{IDC_RELOAD, NSCommandKeyMask, ui::VKEY_R}, {IDC_RELOAD, NSCommandKeyMask, ui::VKEY_R},
{IDC_RELOAD_BYPASSING_CACHE, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_R}, {IDC_RELOAD_BYPASSING_CACHE, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_R},
{IDC_ZOOM_NORMAL, NSCommandKeyMask, ui::VKEY_0}, {IDC_ZOOM_NORMAL, NSCommandKeyMask, ui::VKEY_0},
{IDC_HOME, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_H}, {IDC_HOME, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_H},
{IDC_BACK, NSCommandKeyMask, ui::VKEY_OEM_4}, {IDC_BACK, NSCommandKeyMask, ui::VKEY_OEM_4},
{IDC_FORWARD, NSCommandKeyMask, ui::VKEY_OEM_6}, {IDC_FORWARD, NSCommandKeyMask, ui::VKEY_OEM_6},
{IDC_BOOKMARK_ALL_TABS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_D}, {IDC_BOOKMARK_ALL_TABS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_D},
{IDC_MINIMIZE_WINDOW, NSCommandKeyMask, ui::VKEY_M}, {IDC_MINIMIZE_WINDOW, NSCommandKeyMask, ui::VKEY_M},
{IDC_SELECT_NEXT_TAB, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_RIGHT}, {IDC_SELECT_NEXT_TAB, NSControlKeyMask, ui::VKEY_TAB},
{IDC_SELECT_PREVIOUS_TAB, NSCommandKeyMask | NSAlternateKeyMask, {IDC_SELECT_PREVIOUS_TAB, NSControlKeyMask | NSShiftKeyMask, ui::VKEY_TAB},
ui::VKEY_LEFT}, {IDC_HELP_PAGE_VIA_MENU, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_2},
{IDC_HELP_PAGE_VIA_MENU, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_2},
}; };
// Create a Cocoa platform accelerator given a cross platform |key_code| and // Create a Cocoa platform accelerator given a cross platform |key_code| and
......
...@@ -147,10 +147,20 @@ void SetIsInputSourceDvorakQwertyForTesting(bool is_dvorak_qwerty) { ...@@ -147,10 +147,20 @@ void SetIsInputSourceDvorakQwertyForTesting(bool is_dvorak_qwerty) {
} }
} }
// Clear shift key for printable characters. // [ctr + shift + tab] generates the "End of Medium" keyEquivalent rather than
if ((eventModifiers & (NSNumericPadKeyMask | NSFunctionKeyMask)) == 0 && // "Horizontal Tab". We still use "Horizontal Tab" in the main menu to match
[[self keyEquivalent] characterAtIndex:0] != '\r') // the behavior of Safari and Terminal. Thus, we need to explicitly check for
eventModifiers &= ~NSShiftKeyMask; // this case.
if ((eventModifiers & NSShiftKeyMask) &&
[eventString isEqualToString:@"\x19"]) {
eventString = @"\x9";
} else {
// Clear shift key for printable characters, excluding tab.
if ((eventModifiers & (NSNumericPadKeyMask | NSFunctionKeyMask)) == 0 &&
[[self keyEquivalent] characterAtIndex:0] != '\r') {
eventModifiers &= ~NSShiftKeyMask;
}
}
// Clear all non-interesting modifiers // Clear all non-interesting modifiers
eventModifiers &= NSCommandKeyMask | eventModifiers &= NSCommandKeyMask |
......
...@@ -291,6 +291,13 @@ TEST(NSMenuItemAdditionsTest, TestFiresForKeyEvent) { ...@@ -291,6 +291,13 @@ TEST(NSMenuItemAdditionsTest, TestFiresForKeyEvent) {
key = KeyEvent(0x100108, @"s", @"\u0441", 1); key = KeyEvent(0x100108, @"s", @"\u0441", 1);
ExpectKeyFiresItem(key, MenuItem(@"s", 0x100000), false); ExpectKeyFiresItem(key, MenuItem(@"s", 0x100000), false);
ExpectKeyDoesntFireItem(key, MenuItem(@"c", 0x100000)); ExpectKeyDoesntFireItem(key, MenuItem(@"c", 0x100000));
// ctr + shift + tab produces the "End of Medium" keyEquivalent, even though
// it should produce the "Horizontal Tab" keyEquivalent. Check to make sure
// it matches anyways.
key = KeyEvent(0x60103, @"\x19", @"\x19", 1);
ExpectKeyFiresItem(key, MenuItem(@"\x9", NSShiftKeyMask | NSControlKeyMask),
false);
} }
NSString* keyCodeToCharacter(NSUInteger keyCode, NSString* keyCodeToCharacter(NSUInteger keyCode,
......
...@@ -593,20 +593,6 @@ void ExtractUnderlines(NSAttributedString* string, ...@@ -593,20 +593,6 @@ void ExtractUnderlines(NSAttributedString* string,
if (EventIsReservedBySystem(theEvent)) if (EventIsReservedBySystem(theEvent))
return NO; return NO;
// If we return |NO| from this function, cocoa will send the key event to
// the menu and only if the menu does not process the event to |keyDown:|. We
// want to send the event to a renderer _before_ sending it to the menu, so
// we need to return |YES| for all events that might be swallowed by the menu.
// We do not return |YES| for every keypress because we don't get |keyDown:|
// events for keys that we handle this way.
NSUInteger modifierFlags = [theEvent modifierFlags];
if ((modifierFlags & NSCommandKeyMask) == 0) {
// Make sure the menu does not contain key equivalents that don't
// contain cmd.
DCHECK(![[NSApp mainMenu] performKeyEquivalent:theEvent]);
return NO;
}
// Command key combinations are sent via performKeyEquivalent rather than // Command key combinations are sent via performKeyEquivalent rather than
// keyDown:. We just forward this on and if WebCore doesn't want to handle // keyDown:. We just forward this on and if WebCore doesn't want to handle
// it, we let the WebContentsView figure out how to reinject it. // it, we let the WebContentsView figure out how to reinject it.
...@@ -656,9 +642,6 @@ void ExtractUnderlines(NSAttributedString* string, ...@@ -656,9 +642,6 @@ void ExtractUnderlines(NSAttributedString* string,
if (EventIsReservedBySystem(theEvent)) if (EventIsReservedBySystem(theEvent))
return; return;
DCHECK(eventType != NSKeyDown ||
!equiv == !(modifierFlags & NSCommandKeyMask));
if (eventType == NSFlagsChanged) { if (eventType == NSFlagsChanged) {
// Ignore NSFlagsChanged events from the NumLock and Fn keys as // Ignore NSFlagsChanged events from the NumLock and Fn keys as
// Safari does in -[WebHTMLView flagsChanged:] (of "WebHTMLView.mm"). // Safari does in -[WebHTMLView flagsChanged:] (of "WebHTMLView.mm").
......
...@@ -345,6 +345,16 @@ NSEvent* SynthesizeKeyEvent(NSWindow* window, ...@@ -345,6 +345,16 @@ NSEvent* SynthesizeKeyEvent(NSWindow* window,
NSString* charactersIgnoringModifiers = NSString* charactersIgnoringModifiers =
[[[NSString alloc] initWithCharacters:&shifted_character [[[NSString alloc] initWithCharacters:&shifted_character
length:1] autorelease]; length:1] autorelease];
// Control + [Shift] Tab is special.
if (keycode == ui::VKEY_TAB && (flags & NSControlKeyMask)) {
if (flags & NSShiftKeyMask) {
charactersIgnoringModifiers = @"\x19";
} else {
charactersIgnoringModifiers = @"\x9";
}
}
NSString* characters; NSString* characters;
// The following were determined empirically on OSX 10.9. // The following were determined empirically on OSX 10.9.
if (flags & NSControlKeyMask) { if (flags & NSControlKeyMask) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment