New issue
Advanced search Search tips

Issue 832291 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

custom titlebar: touch input on window controls "leaks" to underlying windows

Project Member Reported by bsep@chromium.org, Apr 12 2018

Issue description

Chrome Version: 67.0.3395.0
OS: Windows 10

1. Open two chrome windows so that their window controls overlap (for example, by maximizing both of them).
2. Tap (do not click) the close button or minimize button.

The action will apply to both windows. This happens even for non-Chrome windows underneath (for example an Explorer window).
 

Comment 1 by bsep@chromium.org, Apr 18 2018

Owner: pbos@chromium.org
Delegating

Comment 2 by pbos@chromium.org, Apr 18 2018

Labels: ReleaseBlock-Stable

Comment 3 by pbos@chromium.org, Apr 18 2018

Labels: M-67

Comment 4 by pbos@chromium.org, Apr 19 2018

Cc: pkasting@chromium.org bsep@chromium.org dtapu...@chromium.org pbos@chromium.org
Labels: -ReleaseBlock-Stable
Owner: ----
Status: Available (was: Assigned)
Looks like some touch events are leaking through as the WM_POINTER events for PT_TOUCH are not marked as handled.

We seem to effectively rely on this to get the compatibility events that are used for handling drag/drop events for tab movement and dragging the window. PT_PEN events are always marked as handled and don't generate compatibility events. I believe catching PT_TOUCH this is the correct thing to do, however:

Marking the PT_TOUCH and PT_PEN events this instead means that tab drag/drop behavior is broken and you cannot drag the Window around (you can drag a tab for exactly one frame, try using a Surface Pen on stable). This seems similar EnableMouseInPointer() causing OLE drag/drop to fail ( https://stackoverflow.com/questions/34218514/drag-drop-stops-working-after-enablemouseinpointer ) as mouse events are delivered as pointer events.

Not sure what to do off the bat here, but it seems non-trivial as breaking touch as badly as PT_PEN here is really bad (can't move window or rearrange tabs) so we might have to live with this for a bit. Normally I'd say that we should consider rolling back custom titlebar but we kind of really need it. I think this might be a good first bug for our new team member who will start in May. I don't think this is blocking anymore as the option to turn custom titlebar isn't attractive either.

+pkasting@ in case you have any ideas on if we could possibly handle drag/drop using WM_POINTER events. I'm unassigning myself but putting it on my backlog hotlist so I don't lose track of it. If you have any ideas w/r/t drag/drop behavior and WM_POINTER we might be able to pick it up.
Cc: robliao@chromium.org
I know nothing at all about touch input or its event types.  Looping in robliao@ who might have more background on this.

Comment 6 by pbos@chromium.org, Apr 19 2018

robliao@ let me know if I can provide any more info. Try using a surface pen as input (or replace https://cs.chromium.org/chromium/src/ui/views/win/hwnd_message_handler.cc?l=2917&rcl=ba177d2ea70e292a846ee0a04117d82f27d674b2 with TRUE to get the same behavior for touch). If we can get drag/drop and window dragging working we can fix the touch-through by marking the events as handled.

Additionally: The rest of the NC area is touch through without minimizing or closing the window. You can highlight (but not trigger) the X button underneath it by touching it.

Comment 7 by pbos@chromium.org, May 9 2018

Owner: bsep@chromium.org
Status: Assigned (was: Available)

Comment 8 by bsep@chromium.org, May 14 2018

Cc: nyerramilli@chromium.org rbasuvula@chromium.org pbomm...@chromium.org
 Issue 842586  has been merged into this issue.

Comment 9 by bsep@chromium.org, May 14 2018

Since I just pinged this, it's the same root cause as  bug 838870 
Project Member

Comment 10 by bugdroid1@chromium.org, May 15 2018

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

commit f99e202ce1f30e12b84d2138fc2d6cd1b32fee79
Author: Bret Sepulveda <bsep@chromium.org>
Date: Tue May 15 01:03:04 2018

Fix IsMouseEventFromTouch not handling non-client mouse events.

Split off from crrev.com/c/1048877. IsMouseEventFromTouch would always
return false for non-client mouse events even if they were from touch.
HWNDMessageHandler lets these fall through anyway so this change alone
does not fix any bugs (as far as I know).

Bug:  838870 ,  832291 
Change-Id: I2fc58816b7586a3347fade9e8171362137a2cbf5
Reviewed-on: https://chromium-review.googlesource.com/1058548
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558565}
[modify] https://crrev.com/f99e202ce1f30e12b84d2138fc2d6cd1b32fee79/ui/events/win/events_win.cc

Project Member

Comment 11 by bugdroid1@chromium.org, May 21 2018

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

commit fdafd691ac38754d9b2e2ef1990d0b478985ac0b
Author: Bret Sepulveda <bsep@chromium.org>
Date: Mon May 21 21:51:49 2018

Fix strange touch behavior on Windows titlebar.

This patch fixes three bugs:
* Dragging the window via finger touch on the titlebar only works about
50% of the time.
* Touching the custom titlebar caption buttons would "leak" to the
window underneath, if the top window was closed or minimized.
* Touching and dragging the custom titlebar caption buttons would
cause further touch input on other widgets (like popup bubbles) to
not work.

This patch fixes them all at once by revising how we handle mouse events
synthesized from touch:
* We were letting all mouse events synthesized from touch fall through
as regular mouse events, unless they were targeted at HTCLIENT. They
would cause problems like setting mouse capture incorrectly (causing bug
#3) and cause double inputs ( bug #2 ). This patch instead marks these
events as handled and ignores them, unless otherwise noted below.
* We now DefWindowProc events targeted at HTCAPTION and HTSYSMENU
instead of simply ignoring them, which fixes bug #1. Our special drag
handling in HandleMouseInputForCaption works poorly for touch.
* We also DefWindowProc events targeted at the caption buttons when
custom titlebar is off, which is required for them to work.
* When we do the hittest for the above targeting, we no longer convert
LPARAM to window coordinates before sending WM_NCHITTEST, because the
point should remain in screen coordinates. This was causing all three
bugs to reproduce inconsistently, since we would sometimes get HTCLIENT
or HTNOWHERE for events clearly targeting non-client area.

This patch doesn't specifically address the broken pen touch case, but
as a side effect it makes dragging the window with the pen work slightly
more often.

Bug:  838870 ,  832291 ,  843486 
Change-Id: Ie7d339f7b1b75acb14377d8411234d8faa16755f
Reviewed-on: https://chromium-review.googlesource.com/1048877
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560353}
[modify] https://crrev.com/fdafd691ac38754d9b2e2ef1990d0b478985ac0b/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/fdafd691ac38754d9b2e2ef1990d0b478985ac0b/ui/views/win/hwnd_message_handler.cc

Comment 12 by bsep@chromium.org, May 21 2018

Status: Fixed (was: Assigned)
Labels: TE-Verified-M68 TE-Verified-68.0.3437.2
Verified this issue on Windows-10 using chrome latest canary #68.0.3437.2 by following steps mentioned in original comment, observed the chrome main window stays open while closing the guest window. Hence marking this issue as TE-Verified for M68.

Thanks! 

Comment 14 by bsep@chromium.org, May 24 2018

 Issue 846103  has been merged into this issue.

Sign in to add a comment