|
|
Created:
3 years, 8 months ago by tapted Modified:
3 years, 8 months ago CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, nona+watch_chromium.org, darin-cc_chromium.org, mac-reviews_chromium.org, James Su, chrome-apps-syd-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMac: Never dispatch keyEquivalents to the web contents if the window is not Key.
When pressing a hotkey on Mac, AppKit asks pretty much everything in the
UI if it wants to handle it. When a dialog is showing, we want the
browser window to handle commands such as "change tabs". So events can
propagate up to a browser window, but WebContents shouldn't see events
when a dialog is active.
In fact, the WebContents doesn't: the dialog machinery invokes
RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is
showing, but RenderWidgetHostImpl ignores the event only *after*
[RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO.
Ignored events are not sent over IPC, so they are never sent back
"unhandled" to be redispatched. So this prevents other parts of the UI
(such as the mainMenu) from participating in event dispatch - the event
is lost.
This became an issue when dialogs started doing their own command
propagation rather than hooking the -[NSWindow _sharesParentKeyState]
SPI which causes a bunch of other weird issues.
To fix, have RenderWidgetHostViewCocoa return NO immediately if its
window is not the keyWindow. It currently just checks if it's the
firstResponder in its window.
BUG=692377
TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to
https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then
refresh the page (dialog should show a second time). Type some text and
press Cmd+x to 'Cut' - text should be cut and Edit menu should flash.
Review-Url: https://codereview.chromium.org/2834763002
Cr-Commit-Position: refs/heads/master@{#466553}
Committed: https://chromium.googlesource.com/chromium/src/+/a9e14964b037e70a90936908b8740d50b73e3b73
Patch Set 1 #Patch Set 2 : fix a test that will probably fail #Patch Set 3 : Add a bespoke test #
Messages
Total messages: 35 (26 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Never perform keyEquivalents on the web contents if the window is not Key BUG=692377 ========== to ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not they keyindow. It currently just checks if its the firstResponder in its window. BUG=692377 ==========
tapted@chromium.org changed reviewers: + thakis@chromium.org
Patchset #3 (id:40001) has been deleted
Description was changed from ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not they keyindow. It currently just checks if its the firstResponder in its window. BUG=692377 ========== to ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 ========== to ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=RenderWidgetHostViewMacTest.ForwardKeyEquivalentsOnlyIfKey ==========
Hi Nico, please take a look
lgtm TEST= were originally for explaining to QA how to manually test what the CL fixes. I think that'd useful for this CL so that future people can understand how to repro what this fixes.
Description was changed from ========== Never dispatch keyEquivalents to the web contents if the window is not Key When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=RenderWidgetHostViewMacTest.ForwardKeyEquivalentsOnlyIfKey ========== to ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show again). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ==========
Description was changed from ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show again). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ========== to ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show a second time). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ==========
On 2017/04/21 14:27:17, Nico wrote: > lgtm > > TEST= were originally for explaining to QA how to manually test what the CL > fixes. I think that'd useful for this CL so that future people can understand > how to repro what this fixes. Done! Thanks Nico.
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
tapted@chromium.org changed reviewers: + asvitkine@chromium.org
+asvitkine for OWNERS - please take a look. Thanks!
lgtm
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs", so events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if its the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show a second time). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ========== to ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs". So events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if it's the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show a second time). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ==========
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492897090242320, "parent_rev": "0c9b90603321c436e6f892be5435fed84eff857a", "commit_rev": "a9e14964b037e70a90936908b8740d50b73e3b73"}
Message was sent while issue was closed.
Description was changed from ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs". So events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if it's the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show a second time). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. ========== to ========== Mac: Never dispatch keyEquivalents to the web contents if the window is not Key. When pressing a hotkey on Mac, AppKit asks pretty much everything in the UI if it wants to handle it. When a dialog is showing, we want the browser window to handle commands such as "change tabs". So events can propagate up to a browser window, but WebContents shouldn't see events when a dialog is active. In fact, the WebContents doesn't: the dialog machinery invokes RenderWidgetHostImpl::SetIgnoreInputEvents(..) while the dialog is showing, but RenderWidgetHostImpl ignores the event only *after* [RenderWidgetHostViewCocoa performKeyEquivalent:] has returned NO. Ignored events are not sent over IPC, so they are never sent back "unhandled" to be redispatched. So this prevents other parts of the UI (such as the mainMenu) from participating in event dispatch - the event is lost. This became an issue when dialogs started doing their own command propagation rather than hooking the -[NSWindow _sharesParentKeyState] SPI which causes a bunch of other weird issues. To fix, have RenderWidgetHostViewCocoa return NO immediately if its window is not the keyWindow. It currently just checks if it's the firstResponder in its window. BUG=692377 TEST=On Mac, Enable chrome://flags/#secondary-ui-md, Navigate to https://httpbin.org/basic-auth/user/passwd, cancel the dialog, then refresh the page (dialog should show a second time). Type some text and press Cmd+x to 'Cut' - text should be cut and Edit menu should flash. Review-Url: https://codereview.chromium.org/2834763002 Cr-Commit-Position: refs/heads/master@{#466553} Committed: https://chromium.googlesource.com/chromium/src/+/a9e14964b037e70a90936908b874... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/a9e14964b037e70a90936908b874... |