ws: Touch selection handles use Env pre-target handlers |
||
Issue description
ws: Touch selection handles use Env pre-target handlers
The text touch handles close when they observe key/mouse events.
When run in ws clients, they miss mouse movements outside their bounds.
On linux-chromeos ToT @ #590830, with two mice plugged in (or on device with touch+mouse/trackpad)
(1) Run chrome --enable-features=SingleProcessMash --touch-devices=<value from 'xinput list'>
(also applies to KSV app Ctrl-Alt-/ without SingleProcessMash)
(2) Click the the omnibox or ksv search textfield
(3) Move the mouse outside the entire window
(4) Type some text in the omnibox/textfield
(5) Tap the text (or use the touch-device mouse to simulate touch) to show handles
(6) Move the mouse slightly, staying outside the window.
Expected: The touch handles disappear
Actual: The touch handles do not disappear
This is because TouchSelectionControllerImpl uses an Env pre-target event handler:
https://cs.chromium.org/chromium/src/ui/views/touchui/touch_selection_controller_impl.cc?rcl=c533f88ff7ad04053e3c05651a9dfb77f2f25ee0&l=421
Mus clients like the KSV (and Chrome in SingleProcessMash) have a separate Env from the Chrome OS system UI (Ash::Shell) env.
I think the same thing applies to web content textfields, because of a similar code pattern:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc?rcl=c533f88ff7ad04053e3c05651a9dfb77f2f25ee0&l=75
But I can't test that at the moment, as touch is broken in SingleProcessMash ( Issue 866991 )
Both can probably be solved with a PointerWatcher to observe system-wide mouse events.
TBD if we need to fix key events, most should be sent to the app with focus, except maybe system keys like power/volume/brightness
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0663980bf2c4f570626981f59d7cfbadd855ba85 commit 0663980bf2c4f570626981f59d7cfbadd855ba85 Author: Mike Wasserman <msw@chromium.org> Date: Thu Sep 20 23:36:30 2018 ws: Add touch selection controller support for pointer watcher. Allows ws clients to close touch ui on events outside their windows. Add a new PointerWatcher plumbing to [Mus]ViewsDelegate. Wires up watchers to the Mus-only PointerWatcherEventRouter. (ui/views/mus/* is not allowed as a dependency of ui/views/*) Destroy the touch selection ui on PointerWatcher mouse events. Bug: 884394 , 887725 Test: Mouse events outside the KSV window close KSV text touch ui. Change-Id: I0e41da4d03719001191f7d5ab669bcc1805bd63c Reviewed-on: https://chromium-review.googlesource.com/1232955 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#593001} [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/base/touch/touch_editing_controller.h [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/mus/mus_views_delegate.cc [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/mus/mus_views_delegate.h [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/touchui/touch_selection_controller_impl.cc [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/touchui/touch_selection_controller_impl.h [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/views_delegate.cc [modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/views_delegate.h
,
Oct 22
This is generally fixed and fits more under Issue 896947. I have some WIP cleanup as part of https://chromium-review.googlesource.com/c/chromium/src/+/1292274
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/acb5a6e01379d536540f967981f7eb04698a157a commit acb5a6e01379d536540f967981f7eb04698a157a Author: Mike Wasserman <msw@chromium.org> Date: Tue Oct 23 20:58:13 2018 mash: Make TouchSelectionControllerClientAura use ui::EventObserver Close touch selection ui in Mash on events outside the browser window. (global pre-target handlers are not supported in window service clients) This follows and refines the new pattern in TouchSelectionControllerImpl. Bug: 896973 , 884394 Test: Text touch selection ui hides as expected in Views and Content. Change-Id: Iedba63a7ce5d5f70f9763cf540d26ee69cee5f28 Reviewed-on: https://chromium-review.googlesource.com/c/1292274 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#602084} [modify] https://crrev.com/acb5a6e01379d536540f967981f7eb04698a157a/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc [modify] https://crrev.com/acb5a6e01379d536540f967981f7eb04698a157a/content/browser/renderer_host/input/touch_selection_controller_client_aura.h [modify] https://crrev.com/acb5a6e01379d536540f967981f7eb04698a157a/ui/views/touchui/touch_selection_controller_impl.cc |
||
►
Sign in to add a comment |
||
Comment 1 by msw@chromium.org
, Sep 14