GitLab

It's been said before and will be said again: keyboard event handling in
Chrome is a monster. Several things came together to make this happen:

First, in most Mac apps, cmd+w closes the dictionary popover if it's
visible. This specifically happens through the `performClose:` action
sent by the main menu. But, in Chrome, cmd+w closes the tab and not the
window, so cmd+w sends a different action. This meant that cmd+w
originally closed the tab and left the dictionary popover floating. This
was fixed many years ago (see r118131 and r322075) by making
`-[AppController menuNeedsUpdate:]` check the current target of
`performClose:` and only map cmd+w to "close tab" when the target is a
browser window.

This would work but for another layer of keyboard shortcut handling
trickery. For a number of reasons (see comment beginning "If this
keyEquivalent" added in r565490), Chrome doesn't actually let
`NSMenuItem`s send their own actions. Instead,
ChromeCommandDispatcherDelegate determines which menu item *would* have
been targeted by a given event, then handles the event itself.

The end result is that, when the dictionary popover is visible, the menu
items are successfully remapped so that cmd+w means "Close Window". But,
any actual cmd+w event is grabbed by ChromeCommandDispatcherDelegate and
sent directly to Chrome's own keyboard shortcut handling… so the browser
window closes.

This change adds a check that was proposed back in r322075, and makes
ChromeCommandDispatcherDelegate only grab the event when the found menu
item's action would be sent to its own window.

Bug: 947752
Change-Id: Ib32988d5fb69c79f899069feb02f7529ea05225f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849431
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705275}