New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 17
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocking:
issue 846893



Sign in to add a comment

macOS: Displayed hotkey for select next/previous tab should be ctr + tab, ctr + shift +tab.

Project Member Reported by erikc...@chromium.org, Jun 11

Issue description

Blocking: 846893
Cc: thakis@chromium.org
Labels: OS-Mac
Summary: macOS: Displayed hotkey for select next/previous tab should be ctr + tab, ctr + shift +tab. (was: macOS: replacing cmd-opt-arrows with cmd-shift-arrows)
Hm. I was testing out Safari's new behavior, which is:

cmd+opt+arrow does nothing. cmd+shift+arrow switches tabs.

The problem [observable in Safari], is that cmd+shift+arrow is also used by text fields to highlight all text from cursor to end. 
  * switching to a new tab page focuses their omnibox, and thus disallows further cmd+shift+arrow navigation.
  * switching to a web contents that has a focused textbook also disallows further cmd+shift+arrow navigation.

The same would be true for Chrome. I think this would be a pretty big regression in terms of usability. In this rare instance, I think we should not be compatible with Safari's behavior.

I still think we should update the menu item text to be ctr + tab and ctr + shift + tab though. 

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94e8be05f18471f2d5eb719c8c14e460523fdeee

commit 94e8be05f18471f2d5eb719c8c14e460523fdeee
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 19 22:27:57 2018

Update MainMenu.xib to use Xcode 8.3.2, macOS 10.13.3.

This CL is not intended to have any behavior change.

We need to update some keyEquivalents in MainMenu.xib, and it is much easier to
update the whole file to use Xcode 8.3.2 than to try to get an instance of Xcode
5.1.1 running on a macOS 10.9.5 device.

We refrain from updating all xibs since most of them are going away with the
migration to Views, and historically, updating xibs has had subtle behavior
changes.

Bug:  851714 
Change-Id: Ibae17d5d645a373f29549d08b9d3c1bf95c0b08a
Reviewed-on: https://chromium-review.googlesource.com/1106897
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568621}
[modify] https://crrev.com/94e8be05f18471f2d5eb719c8c14e460523fdeee/chrome/app/nibs/MainMenu.xib
[modify] https://crrev.com/94e8be05f18471f2d5eb719c8c14e460523fdeee/chrome/app/nibs/PRESUBMIT.py

Wow, that sounds terrible, you're right.

Probably still should support cmd-shift-arrows in addition to our shortcuts though?
The plus of supporting cmd+shift+arrow is that users who expect that will have it function properly.

The minus is that if a user is trying to select text in a text field, but accidentally has the wrong element selected, it will cause a page navigation [e.g., much like backspace to navigate]. 

I would like to avoid introducing this hotkey unless there's a strong reason to do so.
Well, it seems it's the standard tab switching hotkey for non-us layouts (which isn't ctrl-tab/ctrl-shift-tab, which is hard to press).

Interestingly, in Finder, cmd-shift-arrow always switches tabs, even if the find text field has focus. Safari's behavior does make more sense for a browser, but it does have the focus stealing drawback you point out. I suppose in practice the omnibox isn't often focused -- I just went through the ~140 tabs I have currently open on this machine and none of them had the omnibox focused.
new tab page always has omnibox focused
sure but how open do you keep an ntp just hanging around
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c93c3dca594c13251590042f228f8a1029eca985

commit c93c3dca594c13251590042f228f8a1029eca985
Author: erikchen <erikchen@chromium.org>
Date: Wed Jul 11 17:24:50 2018

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: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574211}
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/chrome/app/nibs/MainMenu.xib
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/chrome/browser/global_keyboard_shortcuts_mac.mm
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/chrome/browser/ui/cocoa/accelerators_cocoa.mm
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/chrome/browser/ui/cocoa/nsmenuitem_additions.mm
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/chrome/browser/ui/cocoa/nsmenuitem_additions_unittest.mm
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/content/browser/renderer_host/render_widget_host_view_cocoa.mm
[modify] https://crrev.com/c93c3dca594c13251590042f228f8a1029eca985/ui/events/test/cocoa_test_event_utils.mm

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df8ff0849861817c9fe9bad0453ddb29785d77fa

commit df8ff0849861817c9fe9bad0453ddb29785d77fa
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Jul 11 20:35:41 2018

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

This reverts commit c93c3dca594c13251590042f228f8a1029eca985.

Reason for revert: Suspect this caused a crash on 10.13 (dbg): https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29/3973

Original change's description:
> 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: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574211}

TBR=thakis@chromium.org,asvitkine@chromium.org,erikchen@chromium.org

Change-Id: I4ce9b2e4dac1f0f2fae815528a1523326ac1d134
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  851714 
Reviewed-on: https://chromium-review.googlesource.com/1134098
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574311}
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/chrome/app/nibs/MainMenu.xib
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/chrome/browser/global_keyboard_shortcuts_mac.mm
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/chrome/browser/ui/cocoa/accelerators_cocoa.mm
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/chrome/browser/ui/cocoa/nsmenuitem_additions.mm
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/chrome/browser/ui/cocoa/nsmenuitem_additions_unittest.mm
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/content/browser/renderer_host/render_widget_host_view_cocoa.mm
[modify] https://crrev.com/df8ff0849861817c9fe9bad0453ddb29785d77fa/ui/events/test/cocoa_test_event_utils.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e0fccc527819cb8e09682bc280fb35d4b3fafdd

commit 3e0fccc527819cb8e09682bc280fb35d4b3fafdd
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 12 17:52:19 2018

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

The first CL caused test failures because the browser_test was ignoring the
"shift" key when searching for menu items. The IDC_SELECT_NEXT_TAB and
IDC_SELECT_PREVIOUS_TAB only differ by the presence of "shift", so it cannot be
ignored.

> 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 
> Reviewed-on: https://chromium-review.googlesource.com/1106659
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574211}

Change-Id: Id87f364f5c5b303e8b9668e79f813a1c23376c98
TBR: avi@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1134203
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574628}
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/app/nibs/MainMenu.xib
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/browser/global_keyboard_shortcuts_mac.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/browser/ui/cocoa/accelerators_cocoa.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/browser/ui/cocoa/accelerators_cocoa_browsertest.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/browser/ui/cocoa/nsmenuitem_additions.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/chrome/browser/ui/cocoa/nsmenuitem_additions_unittest.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/content/browser/renderer_host/render_widget_host_view_cocoa.mm
[modify] https://crrev.com/3e0fccc527819cb8e09682bc280fb35d4b3fafdd/ui/events/test/cocoa_test_event_utils.mm

Is this now fixed?
Status: Fixed (was: Started)

Sign in to add a comment