Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.

Committed: https://crrev.com/ceec40ad0ad238b6e37d1c18f478cee122b353b0
Cr-Commit-Position: refs/heads/master@{#400621}

[tapted, 2015-09-18 04:23:01]

tapted@chromium.org changed reviewers:
+ rsesek@chromium.org

[tapted, 2015-09-18 05:58:20]

Hi Robert, WDYT? I can't decide whether this is a hack or not.. Strangely,
there's some mechanism that stops Ctrl+LeftClick getting through already, but
nothing for RightClick.

[Robert Sesek, 2015-09-21 21:52:54]

On 2015/09/18 05:58:20, tapted wrote:
> Hi Robert, WDYT? I can't decide whether this is a hack or not.. Strangely,
> there's some mechanism that stops Ctrl+LeftClick getting through already, but
> nothing for RightClick.

FastResizeView seems like the wrong layer. ChromeEventProcessingWindow maybe?
How does Ctrl+LeftClick get short circuited?

[tapted, 2016-02-10 00:15:01]

tapted@chromium.org changed reviewers:
- rsesek@chromium.org

[tapted, 2016-06-10 01:33:55]

Description was changed from

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. It also
prevents a double-click on the webcontent area zooming the window while
a modal sheet is displayed (note: zoom still works in the titlebar and
tab strip - they're not a descendant view of FastResizeView).

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Right-clicking in the omnibox or
elsewhere shouldn't show a context menu.
==========

to

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. It also
prevents a double-click on the webcontent area zooming the window while
a modal sheet is displayed (note: zoom still works in the titlebar and
tab strip - they're not a descendant view of FastResizeView).

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Right-clicking in the omnibox or
elsewhere shouldn't show a context menu.
==========

[tapted, 2016-06-10 06:42:40]

Description was changed from

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. It also
prevents a double-click on the webcontent area zooming the window while
a modal sheet is displayed (note: zoom still works in the titlebar and
tab strip - they're not a descendant view of FastResizeView).

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Right-clicking in the omnibox or
elsewhere shouldn't show a context menu.
==========

to

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works they're handled by -[NSWindow sendEvent:] and seem to to
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
normally.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

[tapted, 2016-06-10 06:43:48]

Hi Robert - PTAL - I've also added a check to TabStripView to fix
http://crbug.com/617895 (see CL description, comments for details).

On 2015/09/21 21:52:54, Robert Sesek wrote:
> On 2015/09/18 05:58:20, tapted wrote:
> > Hi Robert, WDYT? I can't decide whether this is a hack or not.. Strangely,
> > there's some mechanism that stops Ctrl+LeftClick getting through already,
but
> > nothing for RightClick.
> 
> FastResizeView seems like the wrong layer. ChromeEventProcessingWindow maybe?

This would actually be cool - I could hook in there to have a solution that
fixes both MacViews and Cocoa by leveraging ui/base/cocoa/command_dispatcher.
Unfortunately, I think we still need the window controls to respond normally
when there's an attached sheet. E.g. you can still minimize the parent window
(and resize it). See if you agree with the updated CL description.

> How does Ctrl+LeftClick get short circuited?

It looks like [NSWindow sendEvent] checks early on whether there is an attached
sheet. It then still performs hitTests but it doesn't send any mouseDown events
to the thing that was hit:

== Without attached sheet ==

- FramedBrowserWindow ChromeEventProcessingWindow sendEvent:
- CommandDispatcher CommandDispatcher preSendEvent:
- UnderlayOpenGLHostingWindow NSWindow sendEvent:
- NSEvent NSEvent type
- FramedBrowserWindow NSWindow _runningDocModal
- FramedBrowserWindow NSWindow attachedSheet
- FramedBrowserWindow NSWindow sheetParent
- BrowserCrApplication NSApplication keyWindow
- FramedBrowserWindow NSWindow retain
- FramedBrowserWindow NSWindow _tryWindowDragWithEvent:
- FramedBrowserWindow NSWindow _reallySendEvent:isDelayedEvent:

...hitTests (boring stuff removed):...
 NSThemeFrameTitleTextField NSThemeFrameTitleTextField hitTest:
- _NSThemeWidget NSView hitTest:
- _NSThemeZoomWidget NSView hitTest:
- _NSThemeCloseWidget NSView hitTest:
- FullSizeContentView NSView hitTest:
- AvatarButton NSView hitTest:
- TabStripView NSView hitTest:
- NSView NSView hitTest:
- FastResizeView FastResizeView hitTest:
- NSView NSView hitTest:
- ToolbarView NSView hitTest:
etc.
...
mouseDowns.

== With attached sheet (note it doesn't bother checking sheetParent) ==

- FramedBrowserWindow ChromeEventProcessingWindow sendEvent:
- CommandDispatcher CommandDispatcher preSendEvent:
- UnderlayOpenGLHostingWindow NSWindow sendEvent:
- NSEvent NSEvent type
- FramedBrowserWindow NSWindow _runningDocModal
- FramedBrowserWindow NSWindow attachedSheet
- NSKVONotifying_NSWindow NSWindow _isDocModal
- BrowserCrApplication NSApplication keyWindow
- FramedBrowserWindow NSWindow retain
- FramedBrowserWindow NSWindow _tryWindowDragWithEvent:
- FramedBrowserWindow NSWindow _reallySendEvent:isDelayedEvent:
...
hitTests
but no mouseDowns.

[tapted, 2016-06-10 06:45:08]

Description was changed from

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works they're handled by -[NSWindow sendEvent:] and seem to to
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
normally.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

to

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

[tapted, 2016-06-10 06:45:08]

tapted@chromium.org changed reviewers:
+ rsesek@chromium.org

[tapted, 2016-06-10 06:45:40]

Hi Robert - PTAL - I've also added a check to TabStripView to fix
http://crbug.com/617895 (see CL description, comments for details).

On 2015/09/21 21:52:54, Robert Sesek wrote:
> On 2015/09/18 05:58:20, tapted wrote:
> > Hi Robert, WDYT? I can't decide whether this is a hack or not.. Strangely,
> > there's some mechanism that stops Ctrl+LeftClick getting through already,
but
> > nothing for RightClick.
> 
> FastResizeView seems like the wrong layer. ChromeEventProcessingWindow maybe?

This would actually be cool - I could hook in there to have a solution that
fixes both MacViews and Cocoa by leveraging ui/base/cocoa/command_dispatcher.
Unfortunately, I think we still need the window controls to respond normally
when there's an attached sheet. E.g. you can still minimize the parent window
(and resize it). See if you agree with the updated CL description.

> How does Ctrl+LeftClick get short circuited?

It looks like [NSWindow sendEvent] checks early on whether there is an attached
sheet. It then still performs hitTests but it doesn't send any mouseDown events
to the thing that was hit:

== Without attached sheet ==

- FramedBrowserWindow ChromeEventProcessingWindow sendEvent:
- CommandDispatcher CommandDispatcher preSendEvent:
- UnderlayOpenGLHostingWindow NSWindow sendEvent:
- NSEvent NSEvent type
- FramedBrowserWindow NSWindow _runningDocModal
- FramedBrowserWindow NSWindow attachedSheet
- FramedBrowserWindow NSWindow sheetParent
- BrowserCrApplication NSApplication keyWindow
- FramedBrowserWindow NSWindow retain
- FramedBrowserWindow NSWindow _tryWindowDragWithEvent:
- FramedBrowserWindow NSWindow _reallySendEvent:isDelayedEvent:

...hitTests (boring stuff removed):...
 NSThemeFrameTitleTextField NSThemeFrameTitleTextField hitTest:
- _NSThemeWidget NSView hitTest:
- _NSThemeZoomWidget NSView hitTest:
- _NSThemeCloseWidget NSView hitTest:
- FullSizeContentView NSView hitTest:
- AvatarButton NSView hitTest:
- TabStripView NSView hitTest:
- NSView NSView hitTest:
- FastResizeView FastResizeView hitTest:
- NSView NSView hitTest:
- ToolbarView NSView hitTest:
etc.
...
mouseDowns.

== With attached sheet (note it doesn't bother checking sheetParent) ==

- FramedBrowserWindow ChromeEventProcessingWindow sendEvent:
- CommandDispatcher CommandDispatcher preSendEvent:
- UnderlayOpenGLHostingWindow NSWindow sendEvent:
- NSEvent NSEvent type
- FramedBrowserWindow NSWindow _runningDocModal
- FramedBrowserWindow NSWindow attachedSheet
- NSKVONotifying_NSWindow NSWindow _isDocModal
- BrowserCrApplication NSApplication keyWindow
- FramedBrowserWindow NSWindow retain
- FramedBrowserWindow NSWindow _tryWindowDragWithEvent:
- FramedBrowserWindow NSWindow _reallySendEvent:isDelayedEvent:
...
hitTests
but no mouseDowns.

[Robert Sesek, 2016-06-13 22:36:27]

I think this makes sense to me, but why return self instead of nil for the
override?

[tapted, 2016-06-16 05:16:21]

On 2016/06/13 22:36:27, Robert Sesek wrote:
> I think this makes sense to me, but why return self instead of nil for the
> override?

nil works for tab_strip_view, but not fast_resize_view -- the mouseDowns are not
blocked. Maybe `nil` is a signal to say "keep looking - try my subviews", rather
than "nothing to see here". The hitTest documentation is vague about it. It says

"This method is used primarily by an NSWindow object to determine which view
should receive a mouse-down event. You’d rarely need to invoke this method, but
you might want to override it to have a view object hide mouse-down events from
its subviews. This method ignores hidden views."

but it doesn't say exactly _how_ to override it to hide mouse-down events from
subviews.

[Robert Sesek, 2016-06-17 20:55:39]

A someone hesitant LGTM :)

[tapted, 2016-06-20 00:19:35]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-06-20 00:19:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1354873002/40001

[commit-bot: I haz the power, 2016-06-20 01:03:51]

Description was changed from

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

to

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

[commit-bot: I haz the power, 2016-06-20 01:03:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-20 01:08:27]

Description was changed from

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.
==========

to

==========
Mac: Prevent mouse events reaching subviews while a window has an attached Sheet

It's currently possible to show a context menu for just about anything
in Chrome while the window has a modal sheet displayed (the exception is
the tab strip, which has a check in -[TabView menu]). This makes all
sorts of weird stuff possible, like showing a `Print` dialog for the
webcontents over the supposedly modal `Edit bookmark` dialog.

To fix, have the shared (and mostly boring) ancestor view, class
FastResizeView, override -[NSView hitTest:] to return itself when the
window has an attached sheet. This nerfs the context menus. (note: zoom
still works: that's handled by -[NSWindow sendEvent:] and seems to do
separate hit-testing when a sheet is displayed).

The TabStrip is in the titlebar, not a subview of FastResizeView, so it
also needs to check. This makes the check in -[TabView menu] obsolete.
We can't check "higher up" since the buttons in the window frame should
behave as before.

Note the mainMenu still functions as before. So a print dialog can still
be displayed. Just not via a context menu.

BUG=528871, 617895
TEST=Right-click the bookmarks toolbar and open a modal dialog (e.g.
Edit..., Add Page..., Add Folder...). Then:

- Right-clicking in the omnibox,tab strip or elsewhere shouldn't show a
context menu.
- Middle-clicking shouldn't close a tab.

Committed: https://crrev.com/ceec40ad0ad238b6e37d1c18f478cee122b353b0
Cr-Commit-Position: refs/heads/master@{#400621}
==========

[commit-bot: I haz the power, 2016-06-20 01:08:28]

Patchset 3 (id:??) landed as
https://crrev.com/ceec40ad0ad238b6e37d1c18f478cee122b353b0
Cr-Commit-Position: refs/heads/master@{#400621}