• Leonard Grey's avatar
    Revert "Fix cmd+w closing the browser window instead of just the dictionary popover." · da0f0b78
    Leonard Grey authored
    This reverts commit d3d72b4f.
    
    Reason for revert: crbug/1017263
    
    Original change's description:
    > Fix cmd+w closing the browser window instead of just the dictionary popover.
    > 
    > 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}
    
    TBR=avi@chromium.org,sdy@chromium.org
    
    # Not skipping CQ checks because original CL landed > 1 day ago.
    
    Bug: 947752
    Change-Id: I4d2aef624e111d4e14c4a0d36e7f6a08f2fa08cd
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1876690Reviewed-by: default avatarLeonard Grey <lgrey@chromium.org>
    Commit-Queue: Leonard Grey <lgrey@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#708671}
    da0f0b78
global_keyboard_shortcuts_mac.mm 10.4 KB