CSS Motion Path: animate offset-anchor and offset-position

CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property

BUG=638055
Committed: https://crrev.com/4a69c4cc2f00ee6f4949037f3cbb382bee467f9d
Cr-Commit-Position: refs/heads/master@{#420581}

[Eric Willigers, 2016-09-22 01:49:49]

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

[Eric Willigers, 2016-09-22 01:52:48]

Description was changed from

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.


BUG=638055
==========

to

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property


BUG=638055
==========

[suzyh_UTC10 (ex-contributor), 2016-09-22 03:44:53]

lgtm but I'd feel more comfortable if alancutter did quick sanity check. You
also will still need someone from core/css OWNERS.

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp (right):

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp:186: case
CSSPropertyOffsetPosition:
Alph order: switch with CSSPropertyOffsetRotation.

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp
(right):

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp:655: case
CSSPropertyOffsetPosition:
Optional: Swap with CSSPropertyOffsetRotation to maintain local alph order. The
switch as a whole doesn't seem to follow this ordering, so not sure if it really
matters here.

[Eric Willigers, 2016-09-22 04:32:14]

ericwilligers@chromium.org changed reviewers:
+ alancutter@chromium.org, dstockwell@chromium.org

[Eric Willigers, 2016-09-22 04:32:15]

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp (right):

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp:186: case
CSSPropertyOffsetPosition:
On 2016/09/22 03:44:53, suzyh wrote:
> Alph order: switch with CSSPropertyOffsetRotation.

Done.

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp
(right):

https://codereview.chromium.org/2364503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp:655: case
CSSPropertyOffsetPosition:
On 2016/09/22 03:44:53, suzyh wrote:
> Optional: Swap with CSSPropertyOffsetRotation to maintain local alph order.
The
> switch as a whole doesn't seem to follow this ordering, so not sure if it
really
> matters here.

Done.

[alancutter (OOO until 2018), 2016-09-22 08:05:48]

lgtm with nits.

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/interpolation/offset-anchor-interpolation.html
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/interpolation/offset-anchor-interpolation.html:82:
assertInterpolation({
Use assertNoInterpolation().

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/interpolation/offset-position-interpolation.html
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/interpolation/offset-position-interpolation.html:82:
assertInterpolation({
Use assertNoInterpolation(). Doesn't this fail for transitions as is?

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CSSPositionInterpolationType.h
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CSSPositionInterpolationType.h:29: }
This can just be !value.isPairValue(). InterpolationTypes ideally don't need to
be aware of specific property details.

[Eric Willigers, 2016-09-22 09:54:54]

Patchset #4 (id:60001) has been deleted

[Eric Willigers, 2016-09-22 09:57:06]

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/interpolation/offset-anchor-interpolation.html
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/interpolation/offset-anchor-interpolation.html:82:
assertInterpolation({
On 2016/09/22 08:05:47, alancutter wrote:
> Use assertNoInterpolation().

Done.

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/interpolation/offset-position-interpolation.html
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/interpolation/offset-position-interpolation.html:82:
assertInterpolation({
On 2016/09/22 08:05:47, alancutter wrote:
> Use assertNoInterpolation(). Doesn't this fail for transitions as is?

It didn't as AnimatableLengthPoint never implemented
usesDefaultInterpolationWith. It always "animated" x and y, even if those used
default interpolation.

Now changed.

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CSSPositionInterpolationType.h
(right):

https://codereview.chromium.org/2364503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CSSPositionInterpolationType.h:29: }
On 2016/09/22 08:05:47, alancutter wrote:
> This can just be !value.isPairValue(). InterpolationTypes ideally don't need
to
> be aware of specific property details.

Done.

[Eric Willigers, 2016-09-23 03:40:59]

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

[commit-bot: I haz the power, 2016-09-23 03:41:15]

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

[dstockwell, 2016-09-23 03:46:28]

lgtm

[commit-bot: I haz the power, 2016-09-23 05:02:32]

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

[commit-bot: I haz the power, 2016-09-23 05:02:32]

Dry run: This issue passed the CQ dry run.

[Eric Willigers, 2016-09-23 05:10:39]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2016-09-23 05:10:39]

The patchset sent to the CQ was uploaded after l-g-t-m from suzyh@chromium.org,
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2364503002/#ps80001
(title: "no interpolation to/from auto")

[commit-bot: I haz the power, 2016-09-23 05:10:48]

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, 2016-09-23 05:15:59]

Description was changed from

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property


BUG=638055
==========

to

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property


BUG=638055
==========

[commit-bot: I haz the power, 2016-09-23 05:16:00]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-09-23 05:17:49]

Description was changed from

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property


BUG=638055
==========

to

==========
CSS Motion Path: animate offset-anchor and offset-position

offset-position and offset-anchor are both animatable as
<position>.
https://drafts.fxtf.org/motion-1/#offset-position-property
https://drafts.fxtf.org/motion-1/#offset-anchor-property

BUG=638055

Committed: https://crrev.com/4a69c4cc2f00ee6f4949037f3cbb382bee467f9d
Cr-Commit-Position: refs/heads/master@{#420581}
==========

[commit-bot: I haz the power, 2016-09-23 05:17:49]

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