CSS Motion Path: Web Platform tests for longhand parsing

CSS Motion Path: Web Platform tests for longhand parsing

We introduce web platform tests for parsing of
- offset-position
- offset-path
- offset-distance
- offset-rotate
- offset-anchor

The unshipped properties offset-position and offset-anchor currently
fail a test because we have not yet updated position parsing for
a recent CSS Values spec change (see http://crbug.com/717833).

offset-rotate currently fails a test because we parse 0 as an angle.
This was correct (but leads to shorthand parsing ambiguities) until
https://github.com/w3c/csswg-drafts/issues/1162

offset-path fails some tests because we don't yet support url or
basic-shape || geometry-box shapes.

BUG=722757
Review-Url: https://codereview.chromium.org/2886703002
Cr-Commit-Position: refs/heads/master@{#474881}
Committed: https://chromium.googlesource.com/chromium/src/+/ffcd1042f667a55469747ab51345c699424ef1a3

[Eric Willigers, 2017-05-16 09:13:47]

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

[Eric Willigers, 2017-05-16 09:13:56]

ericwilligers@chromium.org changed reviewers:
+ suzyh@chromium.org

[commit-bot: I haz the power, 2017-05-16 09:14:12]

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-05-16 10:27:56]

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

[commit-bot: I haz the power, 2017-05-16 10:27:57]

Dry run: This issue passed the CQ dry run.

[Eric Willigers, 2017-05-16 11:43:30]

Description was changed from

==========
CSS Motion Path: Web Platform tests for longhand parsing

We introduce web platform tests for parsing of
- offset-position
- offset-path
- offset-distance
- offset-rotate
- offset-anchor

The unshipped properties offset-position and offset-anchor currently
fail a test because we have not yet updated position parsing for
a recent CSS Values spec change (see http://crbug.com/717833).

offset-rotate currently fails a test because we parse 0 as an angle.
This was correct (but leads to shorthand parsing ambiguities) until
https://github.com/w3c/csswg-drafts/issues/1162

offset-path fails some tests because we don't yet support url or
basic-shape || geometry-box shapes.

BUG=722757
==========

to

==========
CSS Motion Path: Web Platform tests for longhand parsing

We introduce web platform tests for parsing of
- offset-position
- offset-path
- offset-distance
- offset-rotate
- offset-anchor

The unshipped properties offset-position and offset-anchor currently
fail a test because we have not yet updated position parsing for
a recent CSS Values spec change (see http://crbug.com/717833).

offset-rotate currently fails a test because we parse 0 as an angle.
This was correct (but leads to shorthand parsing ambiguities) until
https://github.com/w3c/csswg-drafts/issues/1162

offset-path fails some tests because we don't yet support url or
basic-shape || geometry-box shapes.

BUG=722757
==========

[suzyh_UTC10 (ex-contributor), 2017-05-22 01:46:20]

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid-expected.txt
(right):

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid-expected.txt:1:
This is a testharness.js-based test.
I think this is the right way to do this, but please keep an eye on the
automatic export process to make sure that the -expected.txt files do not get
upstreamed along with the tests.

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:6:
<link rel="author" title="Eric Willigers"
href="mailto:ericwilligers@google.com">
@chromium?

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:7:
<link rel="help"
href="https://drafts.fxtf.org/motion-1/#offset-anchor-property">
Add a TODO to change this spec link to the working draft when these changes make
it to that stage? It's not clear to me what the protocol for this is.

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:8:
<script src="/resources/testharness.js"></script>
These tests are missing the <meta name="assert" ... > clauses you included in
the css-transforms-2 tests in https://codereview.chromium.org/2882153004

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/resources/testcommon.js
(right):

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/resources/testcommon.js:1:
'use strict';
My comments on https://codereview.chromium.org/2882153004 for the parsing
testcommon file apply here too, as well as the suggestion to group these tests
under a parsing subdirectory to add structure to the test suite.

[suzyh_UTC10 (ex-contributor), 2017-05-25 01:55:39]

lgtm

I have not stepped through to double-check that the grammars tested are as
described in the test and as specced, but will try to get to it before you
submit this CL. I'm adding my lgtm anyway because I'm not concerned if you do
submit without that.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/resources/parsing-testcommon.js
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/resources/parsing-testcommon.js:13:
}, "e.style['" + property + "'] = " + stringifiedValue + " should set the
value");
I believe you modified this description in the version in
https://codereview.chromium.org/2882153004 to say "set the property value".
Likewise under assert_invalid_value below. I recommend diffing against that
submitted version in case there are any other discrepancies we've missed, just
for consistency.

[suzyh_UTC10 (ex-contributor), 2017-05-25 03:52:58]

I've now cross-checked the spec, the described grammar, and the set of tests.
I've got a couple of comments/questions, but they're all optional.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-anchor-parsing-valid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-anchor-parsing-valid.html:8:
<meta name="assert" content="offset-anchor supports the full grammar 'auto |
<position>'.">
Should there be an 'auto' test? (Likewise for other properties that accept
'auto'.)

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-invalid.html:15:
assert_invalid_value("offset-path", "path('M 20 30 A 60 70 80')");
Consider adding a brief comment why this is invalid. Without being familiar with
the specs it's not obvious why this is invalid while the path specs in the other
test are valid.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-valid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-valid.html:31:
assert_valid_value("offset-path", "inset(10% 20% 30% 40%) border-box");
Should there be an example with both basic-shape and geometry-box in the
opposite order?

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-rotate-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-rotate-parsing-invalid.html:8:
<meta name="assert" content="offset-rotate supports only the grammar '[ auto |
reverse ] || <angle>'.">
Maybe add a test that you can't have both 'auto' and 'reverse' in the value.

[Eric Willigers, 2017-05-25 11:43:12]

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid-expected.txt
(right):

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid-expected.txt:1:
This is a testharness.js-based test.
On 2017/05/22 01:46:19, suzyh_UTC10 wrote:
> I think this is the right way to do this, but please keep an eye on the
> automatic export process to make sure that the -expected.txt files do not get
> upstreamed along with the tests.

Removed two of these now that https://codereview.chromium.org/2888283006/
has landed and we no longer support the deprecated syntax for these properties.

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:6:
<link rel="author" title="Eric Willigers"
href="mailto:ericwilligers@google.com">
On 2017/05/22 01:46:19, suzyh_UTC10 wrote:
> @chromium?

Done.

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:7:
<link rel="help"
href="https://drafts.fxtf.org/motion-1/#offset-anchor-property">
On 2017/05/22 01:46:20, suzyh_UTC10 wrote:
> Add a TODO to change this spec link to the working draft when these changes
make
> it to that stage? It's not clear to me what the protocol for this is.

I haven't across any such TODOs in the web platform tests.

https://codereview.chromium.org/2886703002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/offset-anchor-parsing-invalid.html:8:
<script src="/resources/testharness.js"></script>
On 2017/05/22 01:46:20, suzyh_UTC10 wrote:
> These tests are missing the <meta name="assert" ... > clauses you included in
> the css-transforms-2 tests in https://codereview.chromium.org/2882153004

assert added

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-anchor-parsing-valid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-anchor-parsing-valid.html:8:
<meta name="assert" content="offset-anchor supports the full grammar 'auto |
<position>'.">
On 2017/05/25 03:52:57, suzyh_UTC10 wrote:
> Should there be an 'auto' test? (Likewise for other properties that accept
> 'auto'.)

Done. offset-position and offset-anchor now test auto.

offset-path already tested none and offset-rotate already tested auto.

offset-distance has no auto, the default is 0px.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-invalid.html:15:
assert_invalid_value("offset-path", "path('M 20 30 A 60 70 80')");
On 2017/05/25 03:52:57, suzyh_UTC10 wrote:
> Consider adding a brief comment why this is invalid. Without being familiar
with
> the specs it's not obvious why this is invalid while the path specs in the
other
> test are valid.

Done.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-valid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-path-parsing-valid.html:31:
assert_valid_value("offset-path", "inset(10% 20% 30% 40%) border-box");
On 2017/05/25 03:52:57, suzyh_UTC10 wrote:
> Should there be an example with both basic-shape and geometry-box in the
> opposite order?

Done.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-rotate-parsing-invalid.html
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/offset-rotate-parsing-invalid.html:8:
<meta name="assert" content="offset-rotate supports only the grammar '[ auto |
reverse ] || <angle>'.">
On 2017/05/25 03:52:57, suzyh_UTC10 wrote:
> Maybe add a test that you can't have both 'auto' and 'reverse' in the value.

Done.

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/resources/parsing-testcommon.js
(right):

https://codereview.chromium.org/2886703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/css/motion-1/parsing/resources/parsing-testcommon.js:13:
}, "e.style['" + property + "'] = " + stringifiedValue + " should set the
value");
On 2017/05/25 01:55:38, suzyh_UTC10 wrote:
> I believe you modified this description in the version in
> https://codereview.chromium.org/2882153004 to say "set the property value".
> Likewise under assert_invalid_value below. I recommend diffing against that
> submitted version in case there are any other discrepancies we've missed, just
> for consistency.

Done.

[Eric Willigers, 2017-05-25 11:43:20]

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

[commit-bot: I haz the power, 2017-05-25 11:43:31]

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-05-25 12:58:03]

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

[commit-bot: I haz the power, 2017-05-25 12:58:04]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Eric Willigers, 2017-05-26 00:50:03]

Patchset #4 (id:60001) has been deleted

[Eric Willigers, 2017-05-26 00:50:08]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2017-05-26 00:50:09]

The patchset sent to the CQ was uploaded after l-g-t-m from suzyh@chromium.org
Link to the patchset: https://codereview.chromium.org/2886703002/#ps80001
(title: "test")

[commit-bot: I haz the power, 2017-05-26 00:51:21]

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-05-26 02:20:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1495759808509680,
"parent_rev": "ab4ae0f9d8efe37e7357c8ada6c84b86b56cd1bd", "commit_rev":
"ffcd1042f667a55469747ab51345c699424ef1a3"}

[commit-bot: I haz the power, 2017-05-26 02:21:15]

Description was changed from

==========
CSS Motion Path: Web Platform tests for longhand parsing

We introduce web platform tests for parsing of
- offset-position
- offset-path
- offset-distance
- offset-rotate
- offset-anchor

The unshipped properties offset-position and offset-anchor currently
fail a test because we have not yet updated position parsing for
a recent CSS Values spec change (see http://crbug.com/717833).

offset-rotate currently fails a test because we parse 0 as an angle.
This was correct (but leads to shorthand parsing ambiguities) until
https://github.com/w3c/csswg-drafts/issues/1162

offset-path fails some tests because we don't yet support url or
basic-shape || geometry-box shapes.

BUG=722757
==========

to

==========
CSS Motion Path: Web Platform tests for longhand parsing

We introduce web platform tests for parsing of
- offset-position
- offset-path
- offset-distance
- offset-rotate
- offset-anchor

The unshipped properties offset-position and offset-anchor currently
fail a test because we have not yet updated position parsing for
a recent CSS Values spec change (see http://crbug.com/717833).

offset-rotate currently fails a test because we parse 0 as an angle.
This was correct (but leads to shorthand parsing ambiguities) until
https://github.com/w3c/csswg-drafts/issues/1162

offset-path fails some tests because we don't yet support url or
basic-shape || geometry-box shapes.

BUG=722757

Review-Url: https://codereview.chromium.org/2886703002
Cr-Commit-Position: refs/heads/master@{#474881}
Committed:
https://chromium.googlesource.com/chromium/src/+/ffcd1042f667a55469747ab51345...
==========

[commit-bot: I haz the power, 2017-05-26 02:21:16]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/ffcd1042f667a55469747ab51345...