macOS: Displayed hotkey for select next/previous tab should be ctr + tab, ctr + shift +tab. |
||||
Issue description
,
Jun 19 2018
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.
,
Jun 19 2018
,
Jun 19 2018
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
,
Jun 19 2018
Wow, that sounds terrible, you're right. Probably still should support cmd-shift-arrows in addition to our shortcuts though?
,
Jun 19 2018
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.
,
Jun 19 2018
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.
,
Jun 19 2018
new tab page always has omnibox focused
,
Jun 20 2018
sure but how open do you keep an ntp just hanging around
,
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
,
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
,
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
,
Jul 17
Is this now fixed?
,
Jul 17
|
||||
►
Sign in to add a comment |
||||
Comment 1 by thakis@chromium.org
, Jun 13 2018