New issue
Advanced search Search tips

Issue 851095 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocking:
issue 846893



Sign in to add a comment

macOS: Consider removing non-keycode based global shortcuts.

Project Member Reported by erikc...@chromium.org, Jun 8 2018

Issue description

Back in 2009, we decided to add special case logic for cmd+'}' and cmd+'{' to handle non-US keyboards.

See https://bugs.chromium.org/p/chromium/issues/detail?id=25946.

This was not done to address a specific user concern, but based on a theoretical concern  that users with international keyboards might have. This requires a fair amount of extra logic for handling hotkeys, with special cases built in for 10.8 and 10.9, but not later macOS releases. 

See https://cs.chromium.org/chromium/src/chrome/browser/global_keyboard_shortcuts_mac.mm?l=104

All of our other hotkey handling [including appkit itself] uses vkeycodes rather than characters. It's not clear to me that we should ever be creating character based shortcuts, especially ones that are not modifiable, and not listed in the main menu. This seems more liable to cause confusion for users with international keyboards, especially since they have otherwise no indication that cmd+'{' and cmd+'}' will cause special behavior.

The existence of this parallel [but usually incorrect codepath] caused the developer who added the IDC_DEV_TOOLS_INSPECT hotkey to use the wrong mechanism [character rather than vkeycode]. 

See: https://chromium-review.googlesource.com/c/chromium/src/+/1093301

I propose we convert both of these shortcuts back to using virtual keycodes. This allows us to simplify global_keyboard_shortcuts_mac.mm. If users notice, we should figure out a better mechanism for handling hotkeys + international keyboards [e.g. why are only these 2 shortcuts using characters instead of vkeycodes]. 

thakis: request for comment, since you filed the original feature request.


 
Cc: tapted@chromium.org a...@chromium.org
+ avi, tapted - as they've been reviewing by hotkey changes.
That wasn't based on a theoretical concern, but on the observation that the keyboard shortcuts used to switch tabs with a German or French layout in all mac apps didn't work in Chrome. (They're the keys in the same position as {} are on an US layout, but with a different key on them.)
I was about to write "i.e. as long as e.g. hitting cmd-shift-ü and cmd-shift-+ with a german layout keeps switching tabs (same for french etc), I'm happy" -- but then I went and checked in Safari and Terminal, and these shortcuts no longer work there. (I haven't used a non-US layout since I stopped testing UI stuff.) So I guess removing this sounds fine to me. It'd be super annoying to me if I used a German layout, but I guess I would've already been annoyed when the shortcut stopped working in Terminal, and the intent was to match the OS behavior, and that has changed.
...looks like cmd-opt-arrow left/right no longer works in Safari and Terminal either! WHAT IS GOING ON IN CUPERTINO.

(It still works in Firefox, but that's not a good mac ui role model.)

So I suppose we should use the shortcut listed in our menu to ctrl-tab / ctrl-shift-tab  and remove cmd-opt-arrows as well, as much as that pains me.

How are non-US-layout people supposed to switch tabs with the keyboard though? Aha, cmd-shift-arrow left/right works in Safari, Terminal, Finder (but not in Apple Maps). So we should _add_ support for that shortcut.
(And, unrelated, but omg: cmd-1-9 now switches tabs in Safari instead of opening bookmarks! That's a pretty great change on their end, very tastefully adopted from us.)
simplification sgtm

the change in AppKit/Safari behaviour may be due to an observation at https://crbug.com/434801#c15 - i.e. there's a file,  ${HOME}/Library/Preferences/com.apple.symbolichotkeys.plist , which does "stuff" and over time the amount of stuff Apple populate it with in a new user account seems to have gone down..

The complexity we have now may have been us fighting against this file (or it could be unrelated).

Another possibility is that this file is prepopulated on account creation based on whatever keyboard you have attached at the time... E.g. adding an entry here may cause cmd-shift-ü and cmd-shift-+ to work in Safari again..
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 9 2018

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

commit e4ab96fa5adcda6493d04970b92dd92282fcbf61
Author: Erik Chen <erikchen@chromium.org>
Date: Sat Jun 09 04:21:11 2018

Change all global shortcuts to use virtual keycodes.

The original logic to make two shortcuts cmd + '{' and cmd + '}' use key
characters instead of virtual keycodes was done to match behavior of terminal
and Safari back in 2009 for international keyboards. Both of those apps have
since changed their behavior, but Chrome was not updated.

Bug:  851095 
Change-Id: I3414ca784419b81bb8aacd53aa6cc2a1a8ec8c19
Reviewed-on: https://chromium-review.googlesource.com/1093696
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565849}
[modify] https://crrev.com/e4ab96fa5adcda6493d04970b92dd92282fcbf61/chrome/browser/global_keyboard_shortcuts_mac.h
[modify] https://crrev.com/e4ab96fa5adcda6493d04970b92dd92282fcbf61/chrome/browser/global_keyboard_shortcuts_mac.mm
[modify] https://crrev.com/e4ab96fa5adcda6493d04970b92dd92282fcbf61/chrome/browser/global_keyboard_shortcuts_mac_unittest.mm
[modify] https://crrev.com/e4ab96fa5adcda6493d04970b92dd92282fcbf61/chrome/browser/ui/views/accelerator_table_unittest_mac.mm

Status: Fixed (was: Assigned)

Comment 9 by thakis@chromium.org, Jun 11 2018

Blocking: 846893
Is there a different bug somewhere for

a) Making the menu-blinking shortcut ctrl-(shift)-tab
b) replacing cmd-opt-arrows with cmd-shift-arrows?
> a) Making the menu-blinking shortcut ctrl-(shift)-tab

Right now no shortcuts cause the menu to blink. 
https://bugs.chromium.org/p/chromium/issues/detail?id=851317

> b) replacing cmd-opt-arrows with cmd-shift-arrows?

https://bugs.chromium.org/p/chromium/issues/detail?id=851714

Sign in to add a comment