Pointerevent test cleanup

Pointerevent test cleanup

This CL adds the rest of the pointerevent tests
automations in Chrome as well as cleaning up the
text for a test.
Note that Chrome fails the two finger pan test.

BUG=689669
Review-Url: https://codereview.chromium.org/2681923002
Cr-Commit-Position: refs/heads/master@{#449045}
Committed: https://chromium.googlesource.com/chromium/src/+/7cb979b7a35fae74baaf0d36b5f33b5ccab74a31

[Navid Zolghadr, 2017-02-07 21:23:31]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 21:24:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2017-02-07 21:48:18]

nzolghadr@chromium.org changed reviewers:
+ lanwei@chromium.org, mustaq@chromium.org

[commit-bot: I haz the power, 2017-02-07 22:54:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 22:54:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Navid Zolghadr, 2017-02-08 15:36:21]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 15:36:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 15:38:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 15:38:33]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Navid Zolghadr, 2017-02-08 15:41:55]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 15:42:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 16:50:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 16:50:53]

Dry run: This issue passed the CQ dry run.

[mustaq, 2017-02-08 17:16:13]

LGTM % few remaining double-quotes. Plus a request for a future cleanup...

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:244:
} else
Nit: braces for the last |else| as well?

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:248:
{"source": pointerType,
Omit double-quotes on keys ("source" etc) as in the rest of the file. More
below.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:298:
{"source": pointerType,
Ditto on double-quotes.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js
(right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js:4:
return touchScrollInTarget('#scrollTarget', 'down').then(function() {
Hmm, the html description of the test should say "drag text DOWN" instead of
"scroll text DOWN". The user action is drag, whether that causes scroll or not
depends on page.

Please consider fixing the html, I suspect there are many. Also rename
|touchScrollInTarget| to |touchDragInTarget| in pointerevent_common_input.js.
Perhaps all these should be done in a separate standalone CL for sake of a
better CL description.

[mustaq, 2017-02-08 17:19:30]

... and modulo a crbug ref fix...

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1395: crbug.com/613672 [ Mac ]
external/wpt/pointerevents/compat/pointerevent_touch-action_two-finger_interaction-manual.html
[ Skip ]
For the first one, please use crbug.com/632525 instead. And this should be for
all platforms.

[Navid Zolghadr, 2017-02-08 17:31:57]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 17:32:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2017-02-08 17:36:24]

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1395: crbug.com/613672 [ Mac ]
external/wpt/pointerevents/compat/pointerevent_touch-action_two-finger_interaction-manual.html
[ Skip ]
On 2017/02/08 17:19:30, mustaq wrote:
> For the first one, please use crbug.com/632525 instead. And this should be for
> all platforms.

We talked offline but just for the record here.
The pattern here is that they don't skip the tests if they are failing. They
just add a "-expected" file with failure lines in it (similar to what I have
here in this CL) and that means they expect that output (which contains failure)
for that test. So when we fix the problem in Chrome this test starts failing
until we go back and remove/update the "-expected" file.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:244:
} else
On 2017/02/08 17:16:13, mustaq wrote:
> Nit: braces for the last |else| as well?

Done.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:248:
{"source": pointerType,
On 2017/02/08 17:16:13, mustaq wrote:
> Omit double-quotes on keys ("source" etc) as in the rest of the file. More
> below.

Done.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:298:
{"source": pointerType,
On 2017/02/08 17:16:13, mustaq wrote:
> Ditto on double-quotes.

Done.

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js
(right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js:4:
return touchScrollInTarget('#scrollTarget', 'down').then(function() {
On 2017/02/08 17:16:13, mustaq wrote:
> Hmm, the html description of the test should say "drag text DOWN" instead of
> "scroll text DOWN". The user action is drag, whether that causes scroll or not
> depends on page.
> 
> Please consider fixing the html, I suspect there are many. Also rename
> |touchScrollInTarget| to |touchDragInTarget| in pointerevent_common_input.js.
> Perhaps all these should be done in a separate standalone CL for sake of a
> better CL description.

Regarding the TouchScrollInTarget you are correct. That is why I added
pointerDragInTarget to be the user action in pointerevents_common.js. So
touchScrollInTarget should be completely removed. But there were just too many
instances of it that I thought it might be better to just keep them around and
hopefully soon they will be all removed from here and moved to wpt repo with a
standard form rather than me updating all of them here yet again.

[mustaq, 2017-02-08 17:43:06]

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js
(right):

https://codereview.chromium.org/2681923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_touch-action-inherit_highest-parent-none_touch-manual-automation.js:4:
return touchScrollInTarget('#scrollTarget', 'down').then(function() {
On 2017/02/08 17:36:23, Navid Zolghadr wrote:
> On 2017/02/08 17:16:13, mustaq wrote:
> > Hmm, the html description of the test should say "drag text DOWN" instead of
> > "scroll text DOWN". The user action is drag, whether that causes scroll or
not
> > depends on page.
> > 
> > Please consider fixing the html, I suspect there are many. Also rename
> > |touchScrollInTarget| to |touchDragInTarget| in
pointerevent_common_input.js.
> > Perhaps all these should be done in a separate standalone CL for sake of a
> > better CL description.
> 
> Regarding the TouchScrollInTarget you are correct. That is why I added
> pointerDragInTarget to be the user action in pointerevents_common.js. So
> touchScrollInTarget should be completely removed. But there were just too many
> instances of it that I thought it might be better to just keep them around and
> hopefully soon they will be all removed from here and moved to wpt repo with a
> standard form rather than me updating all of them here yet again.

Acknowledged.

[commit-bot: I haz the power, 2017-02-08 18:35:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 18:35:31]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-02-08 18:40:47]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2017-02-08 18:40:48]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org
Link to the patchset: https://codereview.chromium.org/2681923002/#ps60001
(title: "Remove more double-quotes")

[commit-bot: I haz the power, 2017-02-08 18:41:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 18:48:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486579247850030,
"parent_rev": "b4e9e9a40607504274b3df45e6e545098d475529", "commit_rev":
"7cb979b7a35fae74baaf0d36b5f33b5ccab74a31"}

[commit-bot: I haz the power, 2017-02-08 18:48:55]

Description was changed from

==========
Pointerevent test cleanup

This CL adds the rest of the pointerevent tests
automations in Chrome as well as cleaning up the
text for a test.
Note that Chrome fails the two finger pan test.

BUG=689669
==========

to

==========
Pointerevent test cleanup

This CL adds the rest of the pointerevent tests
automations in Chrome as well as cleaning up the
text for a test.
Note that Chrome fails the two finger pan test.

BUG=689669

Review-Url: https://codereview.chromium.org/2681923002
Cr-Commit-Position: refs/heads/master@{#449045}
Committed:
https://chromium.googlesource.com/chromium/src/+/7cb979b7a35fae74baaf0d36b5f3...
==========

[commit-bot: I haz the power, 2017-02-08 18:48:56]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7cb979b7a35fae74baaf0d36b5f3...