Fix the double-tap to click gesture in touch accessibility mode.

Fix the double-tap to click gesture in touch accessibility mode.

Originally Chrome OS's touch accessibility support was implemented purely
as an event rewriter. Tapping the screen generates a mouse move event,
which announces the object under the finger without clicking it, and then
double-tapping anywhere on the screen generates a click at the last point
where the user tapped. It's also possible to trigger clicks and drags by
more advanced means.

This worked well for touch exploration alone, but it's also possible for
the current accessibility object to move due to a gesture (to move to the
next or previous object on the screen, for example) or due to input focus
moving. In those cases, a double-tap was sending a simulated click to the
wrong point - it was sending it to the last touch exploration point, rather
than to the current accessibility object.

This change fixes this behavior by changing how the touch accessibility
support works.

First, it introduces the concept of the anchor point, which is the point
where the next simulated click will go. Initially this is set via touch
exploration, however now as soon as ChromeVox identifies the object under
the finger, it sends the center point of that object as the new explicit
anchor point, and that point overrides the touch exploration point for the
duration of this touch.

Second, when the user does a discrete gesture to click on the current anchor
point (double-tap, split-tap, or single-tap), it sends a gesture command
to ChromeVox, allowing ChromeVox to directly activate the current object.
However, if the user holds their finger down, it still passes through the
touch events so dragging and long-pressing are possible.

Third, it simplifies "split tap" support where you can tap a second finger
as a shortcut to clicking. That support now only supports a basic click and
not a long-press or drag.

BUG=513713, 613694
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[dmazzoni, 2016-05-25 19:54:54]

Description was changed from

==========
Fix the double-tap to click gesture in touch accessibility mode.

Originally Chrome OS's touch accessibility support was implemented purely
as an event rewriter. Tapping the screen generates a mouse move event,
which announces the object under the finger without clicking it, and then
double-tapping anywhere on the screen generates a click at the last point
where the user tapped. It's also possible to trigger clicks and drags by
more advanced means.

This worked well for touch exploration alone, but it's also possible for
the current accessibility object to move due to a gesture (to move to the
next or previous object on the screen, for example) or due to input focus
moving. In those cases, a double-tap was sending a simulated click to the
wrong point - it was sending it to the last touch exploration point, rather
than to the current accessibility object.

This change fixes this behavior by changing how the touch accessibility
support works.

First, it introduces the concept of the anchor point, which is the point
where the next simulated click will go. Initially this is set via touch
exploration, however now as soon as ChromeVox identifies the object under
the finger, it sends the center point of that object as the new explicit
anchor point, and that point overrides the touch exploration point for the
duration of this touch.

Second, when the user does a discrete gesture to click on the current anchor
point (double-tap, split-tap, or single-tap), it sends a gesture command
to ChromeVox, allowing ChromeVox to directly activate the current object.
However, if the user holds their finger down, it still passes through the
touch events so dragging and long-pressing are possible.

Third, it simplifies "split tap" support where you can tap a second finger
as a shortcut to clicking. That support now only supports a basic click and
not a long-press or drag.

BUG=513713,613694
==========

to

==========
Fix the double-tap to click gesture in touch accessibility mode.

Originally Chrome OS's touch accessibility support was implemented purely
as an event rewriter. Tapping the screen generates a mouse move event,
which announces the object under the finger without clicking it, and then
double-tapping anywhere on the screen generates a click at the last point
where the user tapped. It's also possible to trigger clicks and drags by
more advanced means.

This worked well for touch exploration alone, but it's also possible for
the current accessibility object to move due to a gesture (to move to the
next or previous object on the screen, for example) or due to input focus
moving. In those cases, a double-tap was sending a simulated click to the
wrong point - it was sending it to the last touch exploration point, rather
than to the current accessibility object.

This change fixes this behavior by changing how the touch accessibility
support works.

First, it introduces the concept of the anchor point, which is the point
where the next simulated click will go. Initially this is set via touch
exploration, however now as soon as ChromeVox identifies the object under
the finger, it sends the center point of that object as the new explicit
anchor point, and that point overrides the touch exploration point for the
duration of this touch.

Second, when the user does a discrete gesture to click on the current anchor
point (double-tap, split-tap, or single-tap), it sends a gesture command
to ChromeVox, allowing ChromeVox to directly activate the current object.
However, if the user holds their finger down, it still passes through the
touch events so dragging and long-pressing are possible.

Third, it simplifies "split tap" support where you can tap a second finger
as a shortcut to clicking. That support now only supports a basic click and
not a long-press or drag.

BUG=513713,613694
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2016-05-25 19:56:39]

dmazzoni@chromium.org changed reviewers:
+ asargent@chromium.org, dtseng@chromium.org, oshima@chromium.org

[dmazzoni, 2016-05-25 19:56:40]

David, please review all.

Owners reviews:
oshima: ash/, ui/chromeos/
asargent: chrome/common/extensions/api/

[asargent_no_longer_on_chrome, 2016-05-25 23:31:38]

chrome/common/extensions/api/ lgtm

[oshima, 2016-05-26 22:15:39]

I'm not familiar with touch exploration mode. can you quickly explain/show it to
me offline?

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
File ui/chromeos/touch_exploration_controller_unittest.cc (right):

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller_unittest.cc:964: LOG(ERROR) << "***
SplitTap initial hold ***";
debug msg

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller_unittest.cc:978: LOG(ERROR) << "***
SplitTap press ***";
ditto

[David Tseng, 2016-05-26 22:48:24]

https://codereview.chromium.org/2007863004/diff/1/ui/accessibility/ax_enums.idl
File ui/accessibility/ax_enums.idl (right):

https://codereview.chromium.org/2007863004/diff/1/ui/accessibility/ax_enums.i...
ui/accessibility/ax_enums.idl:477: click,
I like the gesture_finger_count naming scheme better as it fits in with the
other enum values (e.g. double_tap_1).

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
File ui/chromeos/touch_exploration_controller.cc (right):

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller.cc:588: if (anchor_point_state_ ==
ANCHOR_POINT_EXPLICITLY_SET) {
I'm not entirely clear when the explicit vs touch explore state gets set. While
touch exploring, ChromeVox gets hover events and sets the focus rect. So,
depending on how quickly a user double taps thereafter, it could result in
either an explicit or touch explored click?

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller.cc:715: LOG(ERROR) << "**
OnTapTimerFired **";
nit: remove

[dmazzoni, 2016-05-26 22:53:49]

Removed logging statements

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
File ui/chromeos/touch_exploration_controller_unittest.cc (right):

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller_unittest.cc:964: LOG(ERROR) << "***
SplitTap initial hold ***";
On 2016/05/26 22:15:39, oshima wrote:
> debug msg

Done.

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller_unittest.cc:978: LOG(ERROR) << "***
SplitTap press ***";
On 2016/05/26 22:15:39, oshima wrote:
> ditto

Done.

[dmazzoni, 2016-05-26 22:59:24]

https://codereview.chromium.org/2007863004/diff/1/ui/accessibility/ax_enums.idl
File ui/accessibility/ax_enums.idl (right):

https://codereview.chromium.org/2007863004/diff/1/ui/accessibility/ax_enums.i...
ui/accessibility/ax_enums.idl:477: click,
On 2016/05/26 22:48:24, David Tseng wrote:
> I like the gesture_finger_count naming scheme better as it fits in with the
> other enum values (e.g. double_tap_1).

The reason I realized that double_tap is a bad name for this is because there
are lots of different ways to achieve a click:

1. Double-tap
2. Single-tap (tap again after touch exploration)
3. Split-tap (tap second finger while exploring)

All of these should result in the same behavior, I don't think ChromeVox needs
to distinguish between them. That's why I thought "click" might be better.

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
File ui/chromeos/touch_exploration_controller.cc (right):

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller.cc:588: if (anchor_point_state_ ==
ANCHOR_POINT_EXPLICITLY_SET) {
On 2016/05/26 22:48:24, David Tseng wrote:
> I'm not entirely clear when the explicit vs touch explore state gets set.
While
> touch exploring, ChromeVox gets hover events and sets the focus rect. So,
> depending on how quickly a user double taps thereafter, it could result in
> either an explicit or touch explored click?

Yes, that's right.

If you touch-explore and ChromeVox never identifies an object under the cursor,
double-tap will click on wherever your finger was. It's meant mainly as a
failsafe if ChromeVox isn't working, and I think it also does the
less-surprising thing if you just happen to be really fast.

If you touch-explore and ChromeVox does identify an object, ChromeVox's explicit
location takes precendence for the duration of that touch.

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller.cc:715: LOG(ERROR) << "**
OnTapTimerFired **";
On 2016/05/26 22:48:24, David Tseng wrote:
> nit: remove

Done.

[dmazzoni, 2016-05-31 23:01:30]

Friendly ping

[David Tseng, 2016-06-01 00:36:16]

lgtm

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
File ui/chromeos/touch_exploration_controller.cc (right):

https://codereview.chromium.org/2007863004/diff/1/ui/chromeos/touch_explorati...
ui/chromeos/touch_exploration_controller.cc:588: if (anchor_point_state_ ==
ANCHOR_POINT_EXPLICITLY_SET) {
On 2016/05/26 22:59:24, dmazzoni wrote:
> On 2016/05/26 22:48:24, David Tseng wrote:
> > I'm not entirely clear when the explicit vs touch explore state gets set.
> While
> > touch exploring, ChromeVox gets hover events and sets the focus rect. So,
> > depending on how quickly a user double taps thereafter, it could result in
> > either an explicit or touch explored click?
> 
> Yes, that's right.
> 
> If you touch-explore and ChromeVox never identifies an object under the
cursor,
> double-tap will click on wherever your finger was. It's meant mainly as a
> failsafe if ChromeVox isn't working, and I think it also does the
> less-surprising thing if you just happen to be really fast.
> 

This is a little surprising to me because a ChromeVox user would click on
something that was never reflected in terms of output. Seems like something
useful for someone who can see the screen while using ChromeVox but possibly
confusing for a blind user.

> If you touch-explore and ChromeVox does identify an object, ChromeVox's
explicit
> location takes precendence for the duration of that touch.

[oshima, 2016-06-01 01:13:02]

https://codereview.chromium.org/2007863004/diff/20001/ui/chromeos/touch_explo...
File ui/chromeos/touch_exploration_controller.cc (right):

https://codereview.chromium.org/2007863004/diff/20001/ui/chromeos/touch_explo...
ui/chromeos/touch_exploration_controller.cc:520:
new_event->set_root_location_f(anchor_point_);
Native event location must be in host window's coordinate, not native screen
coordinate. This will work fine if the host window is positioned at 0,0 but may
break if not. Can you attach secondary touch display and see if this work?

Also, using host/native coordinate may break when a user changed rotation and/or
scaling. Let me know if you want to discuss this offline.

[dmazzoni, 2016-06-02 16:41:59]

On 2016/06/01 00:36:16, David Tseng wrote:
> This is a little surprising to me because a ChromeVox user would click on
> something that was never reflected in terms of output. Seems like something
> useful for someone who can see the screen while using ChromeVox but possibly
> confusing for a blind user.

I understand, maybe we should revisit if this ends up being a problem in
practice. I guess my worry is that I don't want the responsiveness of
ChromeVox to be a single point of failure for the whole system. If
ChromeVox is stuck or frozen it should be possible to still send
touch screen events.

[dmazzoni, 2016-06-02 16:54:03]

https://codereview.chromium.org/2007863004/diff/20001/ui/chromeos/touch_explo...
File ui/chromeos/touch_exploration_controller.cc (right):

https://codereview.chromium.org/2007863004/diff/20001/ui/chromeos/touch_explo...
ui/chromeos/touch_exploration_controller.cc:520:
new_event->set_root_location_f(anchor_point_);
On 2016/06/01 01:13:02, oshima wrote:
> Native event location must be in host window's coordinate, not native screen
> coordinate. This will work fine if the host window is positioned at 0,0 but
may
> break if not. Can you attach secondary touch display and see if this work?
> 
> Also, using host/native coordinate may break when a user changed rotation
and/or
> scaling. Let me know if you want to discuss this offline.

It turns out a few other things are broken when using a second
display. I can't easily test this until I fix other issues.
OK to land this with a TODO and then address all of the
second display issues in a separate patch?

I filed http://crbug.com/616793 and added the TODO.

[oshima, 2016-06-02 18:41:36]

sg lgtm.

[David Tseng, 2016-06-02 19:20:51]

Still lgtm; just replying below:
On 2016/06/02 16:41:59, dmazzoni wrote:
> On 2016/06/01 00:36:16, David Tseng wrote:
> > This is a little surprising to me because a ChromeVox user would click on
> > something that was never reflected in terms of output. Seems like something
> > useful for someone who can see the screen while using ChromeVox but possibly
> > confusing for a blind user.
> 
> I understand, maybe we should revisit if this ends up being a problem in
> practice. I guess my worry is that I don't want the responsiveness of
> ChromeVox to be a single point of failure for the whole system. If
> ChromeVox is stuck or frozen it should be possible to still send
> touch screen events.

That's fine, but it should always be clear (in speech and in braille) what I
clicked on. If it's not or not possible to do so, then we should fix that rather
than making the user do a pass through.

[dmazzoni, 2016-06-03 17:04:09]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-06-03 17:04:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2007863004/#ps40001
(title: "Add TODO for second display")

[commit-bot: I haz the power, 2016-06-03 17:04:33]

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

[commit-bot: I haz the power, 2016-06-03 19:52:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-03 19:54:01]

Description was changed from

==========
Fix the double-tap to click gesture in touch accessibility mode.

Originally Chrome OS's touch accessibility support was implemented purely
as an event rewriter. Tapping the screen generates a mouse move event,
which announces the object under the finger without clicking it, and then
double-tapping anywhere on the screen generates a click at the last point
where the user tapped. It's also possible to trigger clicks and drags by
more advanced means.

This worked well for touch exploration alone, but it's also possible for
the current accessibility object to move due to a gesture (to move to the
next or previous object on the screen, for example) or due to input focus
moving. In those cases, a double-tap was sending a simulated click to the
wrong point - it was sending it to the last touch exploration point, rather
than to the current accessibility object.

This change fixes this behavior by changing how the touch accessibility
support works.

First, it introduces the concept of the anchor point, which is the point
where the next simulated click will go. Initially this is set via touch
exploration, however now as soon as ChromeVox identifies the object under
the finger, it sends the center point of that object as the new explicit
anchor point, and that point overrides the touch exploration point for the
duration of this touch.

Second, when the user does a discrete gesture to click on the current anchor
point (double-tap, split-tap, or single-tap), it sends a gesture command
to ChromeVox, allowing ChromeVox to directly activate the current object.
However, if the user holds their finger down, it still passes through the
touch events so dragging and long-pressing are possible.

Third, it simplifies "split tap" support where you can tap a second finger
as a shortcut to clicking. That support now only supports a basic click and
not a long-press or drag.

BUG=513713,613694
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix the double-tap to click gesture in touch accessibility mode.

Originally Chrome OS's touch accessibility support was implemented purely
as an event rewriter. Tapping the screen generates a mouse move event,
which announces the object under the finger without clicking it, and then
double-tapping anywhere on the screen generates a click at the last point
where the user tapped. It's also possible to trigger clicks and drags by
more advanced means.

This worked well for touch exploration alone, but it's also possible for
the current accessibility object to move due to a gesture (to move to the
next or previous object on the screen, for example) or due to input focus
moving. In those cases, a double-tap was sending a simulated click to the
wrong point - it was sending it to the last touch exploration point, rather
than to the current accessibility object.

This change fixes this behavior by changing how the touch accessibility
support works.

First, it introduces the concept of the anchor point, which is the point
where the next simulated click will go. Initially this is set via touch
exploration, however now as soon as ChromeVox identifies the object under
the finger, it sends the center point of that object as the new explicit
anchor point, and that point overrides the touch exploration point for the
duration of this touch.

Second, when the user does a discrete gesture to click on the current anchor
point (double-tap, split-tap, or single-tap), it sends a gesture command
to ChromeVox, allowing ChromeVox to directly activate the current object.
However, if the user holds their finger down, it still passes through the
touch events so dragging and long-pressing are possible.

Third, it simplifies "split tap" support where you can tap a second finger
as a shortcut to clicking. That support now only supports a basic click and
not a long-press or drag.

BUG=513713,613694
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-06-03 19:54:02]

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