mac triage: erikchen@, can you take a peek at this?

Does this work on Chrome Beta? 

Should be fixed in https://bugs.chromium.org/p/chromium/issues/detail?id=811921

Installed Chrome Beta 70.0.3538.35 today but the issue persists :(

Thanks, I've confirmed this issue. Safari behaves as "expected" -- for some definition of expected. Safari matches cmd + '+' to "switch to tab 1" rather than "zoom in".

Observations.

1) Notice that [cmd + shift + 1] will select the first tab.

Here's an example of the NSEvent that gets generated from [cmd + 1]: NSEvent: type=KeyDown loc=(1547,978) time=1921381.9 flags=0x100108 win=0x7f9284fb1e30 winNum=180473 ctxt=0x0 chars="1" unmodchars="+" repeat=0 keyCode=18

Compare to [cmd + 4]: NSEvent: type=KeyDown loc=(1543,1050) time=1921378.4 flags=0x100108 win=0x7f9284fb1e30 winNum=180473 ctxt=0x0 chars="4" unmodchars="č" repeat=0 keyCode=21

When we process cmd + 4, we start with unmodchars, notice that it's not ascii, and then switch to "chars". This is "4", and everything works as expected.

When we process cmd + 1, we start with unmodchars, notice that it is ascii "+" and stop. This happens to match an existing menu item: "Zoom In" -- [cmd + '+']. We let the event pass through to Cocoa, but Cocoa *fails* to match this against the menu item. 

2) As far as I can tell, there is no way to trigger the "Zoom In" menu command from the Czech keyboard layout in Safari [or using Cocoa menuitem matching]. This seems like a Cocoa bug.

Given that [cmd + shift + 1] does the right thing, I'm disinclined to add a special workaround here. The real bug seems to be that cmd + '+' doesn't trigger 'Zoom In', which is a long-standing Cocoa issue.


+thakis to comment.

comment


...how/why did this work in m68?

In M68 we had different logic for matching unlisted shortcuts which used vkey_code matching. 

This was changed over the course of 2 CLs:
1) https://chromium-review.googlesource.com/c/chromium/src/+/1093696/5/chrome/browser/global_keyboard_shortcuts_mac.mm#154
2) https://chromium-review.googlesource.com/c/chromium/src/+/1167466

For number keys, vkey_codes match characters fairly often. Thus, this change went mostly unnnoticed. However, for non-number key shortcuts, vkey_code and characters are frequently wildly different. This caused bugs on dvorak-qwerty and really most keyboard layouts for letter based hidden shortcuts.

Maybe we can do ...something... by looking at both vkey_code and character somehow?

Looking though https://en.wikipedia.org/wiki/QWERTZ https://en.wikipedia.org/wiki/QWERTY , czech seems to be the only layout that has symbols on the number row by default and you only get numbers if you hold down shift.

Vietnamese has this for numbers 1-4 -- are we broken there too?

I've confirmed that this also doesn't work for the Vietnamese - Vietnamese virtual keyboard. 

It's not clear to me why pressing the key combination

cmd + '+' [vkeycode = keycode for '1'] should trigger the keyEquivalent for cmd + '1' rather than cmd + '+'. 

Given that there are other cases [e.g. dvorak-qwerty] where we want the opposite behavior.

For why, just picture yourself using chrome with a czech keyboard :-) You'd have to use cmd-shift-"1"-"9" to switch tabs, which seems pretty annoying to type if it works differently a) with other apps on the same os b) with the same app on other OSs.

For me, Cmd + Shift + '+' (I think that means Cmd + '1' on Czech keyboard) doesn't switch to tab 1. It may have something to do with the fact, that macOS uses Cmd + Shift + '3' and Cmd + Shift + '4' for screenshots.

Another fact is that when I boot into Linux, where I have the exact same keyboard layout, everything works as expected (Cmd + '+' switches to tab 1). Not sure if there is a way to zoom in though. However, I do not use zoom in on a daily basis in comparison to tab switching.

I agree with the above comment, please leave Cmd + '+' as a tab 1 shortcut. Maybe Cmd + Shift + '+' could be zoom in?

If you provide me with a step by step guide, I am of course willing to provide every possible log I can to solve this issue.

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

commit 2fc6dc8cb9e1587adacdabdb6ebe75070c9def92
Author: erikchen <erikchen@chromium.org>
Date: Wed Oct 03 18:17:43 2018

macOS: Treat cmd + '+' as cmd + '1' on Czech keyboard layouts.

This results in more intuitive behavior of Chrome with respect to tab switching
shortcuts.

Bug:  889424 
Change-Id: I1ccba5141a3ddbfd2391d2d40e0d5884b50a382f
Reviewed-on: https://chromium-review.googlesource.com/1259244
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596288}
[modify] https://crrev.com/2fc6dc8cb9e1587adacdabdb6ebe75070c9def92/chrome/browser/ui/cocoa/nsmenuitem_additions.h
[modify] https://crrev.com/2fc6dc8cb9e1587adacdabdb6ebe75070c9def92/chrome/browser/ui/cocoa/nsmenuitem_additions.mm
[modify] https://crrev.com/2fc6dc8cb9e1587adacdabdb6ebe75070c9def92/chrome/browser/ui/cocoa/nsmenuitem_additions_unittest.mm

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

commit fe60fb8d3235aa1d3c9f3660e1aa53ec5dbc2527
Author: erikchen <erikchen@chromium.org>
Date: Wed Oct 03 18:46:40 2018

Fix vietnamese layout to correctly handle cmd + 1.

In vietnamese layout, cmd + [vkeycode=18] does not print any characters. This
should still be correctly interpretted as a hotkey for cmd + '1'.

Previously, the logic for cr_firesForKeyEvent would early exit when
charactersIgnoringModifiers had 0 length. Subsequent logic would use characters
in place of charactersIgnoringModifiers. This CL moves the latter logic before
the former logic so that if charactersIgnoringModifiers has 0 length, we'll
instead check characters.

Change-Id: I3fcc45579c7697ca843ef3472fa294fa2da02262
Bug:  889424 
Reviewed-on: https://chromium-review.googlesource.com/1259342
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596298}
[modify] https://crrev.com/fe60fb8d3235aa1d3c9f3660e1aa53ec5dbc2527/chrome/browser/ui/cocoa/nsmenuitem_additions.mm
[modify] https://crrev.com/fe60fb8d3235aa1d3c9f3660e1aa53ec5dbc2527/chrome/browser/ui/cocoa/nsmenuitem_additions_unittest.mm

Thank you :)

 Issue 897913  has been merged into this issue.

I am seeing the same bug for French AZERTY layout.
It used to work since forever, and stopped working a couple of weeks ago.

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

 Issue 905616  has been merged into this issue.