New issue
Advanced search Search tips

Issue 872891 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 841020



Sign in to add a comment

ws: aura::WindowTargeter hit test offsets are not respected

Project Member Reported by msw@chromium.org, Aug 9

Issue description

ws: aura::WindowTargeter hit test offsets are not respected

Shortcut Viewer app doesn't support some textfield touch interactions
(1) Run Chrome OS ToT (eg. #581769), running on linux desktop is ok.
    (on linux desktop I'm using --touch-devices=10 --show-taps)
(2) Launch the Keyboard Shortcut Viewer app with Ctrl-Alt-/
(3) Type in the search textfield
(4) Double tap somewhere in the typed text
Expected: The second tap is targeted at the textfield.
Actual: The second tap is targeted at the touch handle window.

AFAICT, TouchHandleWindowTargeter's hit test offsets are not respected.
This aura::WindowTargeter is installed on EditingHandleView windows to shift the hit-test target down.
It's unclear if the window server must respect this or if client-side logic is sufficient.
 
It seems like EasyResizeWindowTargeter was doing something similar by calling the service's WindowTree::SetHitTestMask, but that's not implemented in ws2's WindowTree... I wonder if that's broken, or if the service-side aspect of that feature is no longer needed?
Is this a client-app-side problem or an ash/window-service problem?

I've been working on select-to-speak for KSV. It does an ash/browser side lookup for the target window:
  aura::Window* window =
      root_window->GetEventHandlerForPoint(action.target_point);
  if (!window)
    return;

  // Convert point to local coordinates of the hit window.
  aura::Window::ConvertPointToTarget(root_window, window, &action.target_point);


Then sends a AX event over mojo with a position, then does a hit-test in the client-app via:
widget_->GetRootView()->GetEventHandlerForPoint(action.target_point);

That should all still work OK after fixing this issue, right?

Labels: Proj-Mash-KSV M-70
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14

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

commit 869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a
Author: Mike Wasserman <msw@chromium.org>
Date: Tue Aug 14 21:17:54 2018

ws: Implement WindowTree::SetHitTestMask for KSV touch editing handles.

Add a simple WindowTree::SetHitTestMask impl and unit test.
Wire up the TouchHandleWindowTargeter, used in the KSV app, etc.
Clarify that the targeter only insets bounds, it does not extend them.

Bug:  872891 
Test: KSV app text touches work as expected (modulo  Issue 873743 ).
Change-Id: I4a1b79a8826a7e176ce3dafb29e1223ddd003966
Reviewed-on: https://chromium-review.googlesource.com/1171218
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583034}
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/server_window.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/server_window.h
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/window_tree.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/window_tree.h
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/window_tree_test_helper.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/window_tree_test_helper.h
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/services/ui/ws2/window_tree_unittest.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/ui/aura/mus/window_port_mus.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/ui/aura/mus/window_port_mus.h
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/869ae46b87c56fc7ad9e1e5fc28fa07b10943e6a/ui/views/touchui/touch_selection_controller_impl.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 14

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

commit d216c7a15eb1035e6e67af21fd92543d0aed4bf0
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Sep 14 21:22:48 2018

ws: Apply hit test insets to touch edit handle client roots.

Touch selection editing handles use window targeting insets.
(to let users more easily tap text above the visible handle)

Adjust targeting of the handle's client root window in Mus.
(otherwise events target the server's invisible root window)

Bug:  872891 
Test: KSV (Ctrl-Alt-/) touch edit handles don't block tapping text.
Change-Id: I6d8614257dc4927a1562bc02c2eefef1f8ff2faa
Reviewed-on: https://chromium-review.googlesource.com/1226877
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591478}
[modify] https://crrev.com/d216c7a15eb1035e6e67af21fd92543d0aed4bf0/ui/views/touchui/touch_selection_controller_impl.cc

Sign in to add a comment