Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 149363002: Web Animations API: Implement step-middle and steps(x, middle) timing functions. (Closed)

Created:
6 years, 8 months ago by rjwright
Modified:
6 years, 6 months ago
CC:
blink-reviews, ed+blinkwatch_opera.com, shans, alancutter (OOO until 2018), Mike Lawther (Google), dglazkov+blink, dstockwell, Timothy Loh, apavlov+blink_chromium.org, darktears, Steve Block, dino_apple.com, Eric Willigers
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Web Animations API: Implement step-middle and steps(x, middle) timing functions. step-middle and steps(x, middle) need to work in the Web Animations API but not in CSS. To achieve that we check for step-middle and steps(x, middle) in CSSToStyleMap::mapAnimationTimingFunction, where CSS-land timing functions are set. Step middle functions won’t behave like other invalid inputs; when an animation is made with timing-function: step-middle, or the timing function is set to step-middle after creating the animation, the timing-function value is set to “ease”. In AbstractPropertySetCSSStyleDeclaration::setPropertyInternal (in PropertySetCSSStyleDeclaration.cpp) there is no good way to check the value of the property to be set, so we just don’t. This means that if you do element.style.animationTimingFunction = 'step-middle'; then element.style.animationTimingFunciton will return “step-middle”, but “ease” is used as the actual timing function. This issue is partially mitigated by the fact that we also check RuntimeEnabledFeatures::webAnimationsAPIEnabled at the parsing layer, and don’t parse step middle functions if it’s false. This CL also removes TimingFunctionTestHelper and TimingFunctionTestHelperTest and adds TimingFunctionTest. BUG=327564 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168500

Patch Set 1 #

Patch Set 2 : Remove class in BisonCSSParserTest and change to new license text #

Total comments: 2

Patch Set 3 : Rebase #

Patch Set 4 : Add comment to timing-functions test re. invalidity of step-middle in CSS #

Total comments: 12

Patch Set 5 : Upload after rebase #

Patch Set 6 : Remove bool argument from parseAnimationTimingFunctionValue #

Total comments: 2

Patch Set 7 : Fix TimingFunctionTestHelperTest #

Total comments: 10

Patch Set 8 : Rebase. Change handling of step-middle in CSS. See issue description. #

Patch Set 9 : Remove TimingFunctionTestHelper #

Patch Set 10 : Fix comment on macro #

Patch Set 11 : Rebased. (Still need to test TimingFunction::evaluate, and layout-test step-middle). #

Patch Set 12 : Add unit test cases for TimingFunction::evaluate methods #

Patch Set 13 : Small change to timing-functions.html. See code comment. #

Patch Set 14 : Fix comment typo #

Total comments: 4

Patch Set 15 : Synced and Rebased #

Patch Set 16 : Export equals operators #

Patch Set 17 : Merged patch into fresh branch (to avoid scary rebase) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+631 lines, -603 lines) Patch
M LayoutTests/animations/animations-parsing.html View 1 2 3 4 5 1 chunk +12 lines, -0 lines 0 comments Download
M LayoutTests/animations/animations-parsing-expected.txt View 1 2 3 4 5 1 chunk +8 lines, -0 lines 0 comments Download
M LayoutTests/animations/timing-functions.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +42 lines, -0 lines 0 comments Download
M LayoutTests/animations/timing-functions-expected.txt View 3 chunks +6 lines, -0 lines 0 comments Download
M Source/core/animation/AnimationTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/animation/AnimationTimingInputTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +5 lines, -6 lines 0 comments Download
M Source/core/animation/CompositorAnimationsTest.cpp View 1 2 3 4 5 6 7 8 9 10 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/animation/CompositorAnimationsTimingFunctionReverserTest.cpp View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -22 lines 0 comments Download
M Source/core/animation/TimedItemCalculationsTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/core.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSComputedStyleDeclaration.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/css/CSSTimingFunctionValue.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +7 lines, -6 lines 0 comments Download
M Source/core/css/CSSTimingFunctionValue.cpp View 1 2 3 4 1 chunk +18 lines, -2 lines 0 comments Download
M Source/core/css/CSSValueKeywords.in View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
A Source/core/css/parser/BisonCSSParserTest.cpp View 1 2 3 4 5 1 chunk +67 lines, -0 lines 0 comments Download
M Source/core/css/parser/CSSPropertyParser.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +18 lines, -5 lines 0 comments Download
M Source/core/css/resolver/CSSToStyleMap.cpp View 1 2 3 4 5 6 7 8 9 10 3 chunks +13 lines, -3 lines 0 comments Download
M Source/platform/animation/TimingFunction.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 8 chunks +28 lines, -14 lines 0 comments Download
M Source/platform/animation/TimingFunction.cpp View 1 2 3 4 5 6 7 8 3 chunks +91 lines, -2 lines 0 comments Download
A Source/platform/animation/TimingFunctionTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +308 lines, -0 lines 0 comments Download
D Source/platform/animation/TimingFunctionTestHelper.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -58 lines 0 comments Download
M Source/platform/animation/TimingFunctionTestHelper.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -150 lines 0 comments Download
M Source/platform/animation/TimingFunctionTestHelperTest.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -326 lines 0 comments Download
M Source/platform/blink_platform.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 47 (0 generated)
rjwright
ptal :)
6 years, 8 months ago (2014-01-29 07:26:06 UTC) #1
rjwright
6 years, 8 months ago (2014-01-30 00:52:44 UTC) #2
shans
lgtm needs review from someone familiar with the parser. https://codereview.chromium.org/149363002/diff/20001/LayoutTests/animations/animations-parsing.html File LayoutTests/animations/animations-parsing.html (right): https://codereview.chromium.org/149363002/diff/20001/LayoutTests/animations/animations-parsing.html#newcode291 LayoutTests/animations/animations-parsing.html:291: ...
6 years, 8 months ago (2014-01-30 09:55:08 UTC) #3
dstockwell
lgtm I'm not sure about the flag in the parser, but can't think of a ...
6 years, 7 months ago (2014-02-03 01:18:18 UTC) #4
Timothy Loh
https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode1146 Source/core/css/parser/BisonCSSParser-in.cpp:1146: bool BisonCSSParser::parseValue(MutableStylePropertySet* declaration, CSSPropertyID propertyID, const String& string, bool ...
6 years, 7 months ago (2014-02-03 01:49:48 UTC) #5
rjwright
https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode1146 Source/core/css/parser/BisonCSSParser-in.cpp:1146: bool BisonCSSParser::parseValue(MutableStylePropertySet* declaration, CSSPropertyID propertyID, const String& string, bool ...
6 years, 7 months ago (2014-02-03 02:47:13 UTC) #6
alancutter (OOO until 2018)
https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode4458 Source/core/css/parser/BisonCSSParser-in.cpp:4458: } The above 8 lines might be cleaner as ...
6 years, 7 months ago (2014-02-04 01:19:11 UTC) #7
alancutter (OOO until 2018)
https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/60001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode1146 Source/core/css/parser/BisonCSSParser-in.cpp:1146: bool BisonCSSParser::parseValue(MutableStylePropertySet* declaration, CSSPropertyID propertyID, const String& string, bool ...
6 years, 7 months ago (2014-02-04 02:11:15 UTC) #8
eseidel
I'm about to get on a plane, but as of tomorrow I"ll be on your ...
6 years, 7 months ago (2014-02-05 01:41:17 UTC) #9
eseidel
I guess I don't understand quite what this CL is trying to do, and why ...
6 years, 7 months ago (2014-02-07 00:12:34 UTC) #10
rjwright
Just uploading after small changes + rebase. Still need to sort out that bool arg ...
6 years, 7 months ago (2014-02-17 07:16:52 UTC) #11
rjwright
This works, except that if you do elem.style.animationTimingFunction = 'step-middle'; console.log(elem.style.animationTimingFunction); with experimental features on ...
6 years, 7 months ago (2014-02-19 02:37:51 UTC) #12
alancutter (OOO until 2018)
https://codereview.chromium.org/149363002/diff/510001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/510001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode4364 Source/core/css/parser/BisonCSSParser-in.cpp:4364: return 0; Avoiding else branches by returning earlier is ...
6 years, 7 months ago (2014-02-19 03:17:44 UTC) #13
dstockwell
Need to update description about how step-middle has been disabled for CSS and the quirk ...
6 years, 7 months ago (2014-02-24 04:58:39 UTC) #14
rjwright
https://codereview.chromium.org/149363002/diff/510001/Source/core/css/parser/BisonCSSParser-in.cpp File Source/core/css/parser/BisonCSSParser-in.cpp (right): https://codereview.chromium.org/149363002/diff/510001/Source/core/css/parser/BisonCSSParser-in.cpp#newcode4364 Source/core/css/parser/BisonCSSParser-in.cpp:4364: return 0; On 2014/02/19 03:17:45, alancutter wrote: > Avoiding ...
6 years, 7 months ago (2014-02-24 11:17:46 UTC) #15
rjwright
Test added.
6 years, 7 months ago (2014-02-28 03:48:14 UTC) #16
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 7 months ago (2014-03-03 05:18:13 UTC) #17
dstockwell
lgtm
6 years, 7 months ago (2014-03-03 05:18:16 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1060001
6 years, 7 months ago (2014-03-03 05:18:26 UTC) #19
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 7 months ago (2014-03-03 05:32:57 UTC) #20
rjwright
On 2014/03/03 05:32:57, rjwright wrote: > The CQ bit was unchecked by mailto:rjwright@chromium.org Missing platform ...
6 years, 7 months ago (2014-03-03 06:54:39 UTC) #21
eseidel
I feel like you've just gotten the run-around on this patch set, and we should ...
6 years, 7 months ago (2014-03-03 07:10:47 UTC) #22
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 7 months ago (2014-03-03 08:31:56 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1060001
6 years, 7 months ago (2014-03-03 08:32:09 UTC) #24
rjwright
I'm going to land this as I think dstockwell is going to need it soon. ...
6 years, 7 months ago (2014-03-03 08:38:33 UTC) #25
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-03-03 09:34:09 UTC) #26
commit-bot: I haz the power
Retried try job too often on mac_layout for step(s) webkit_lint http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout&number=21605
6 years, 7 months ago (2014-03-03 09:34:10 UTC) #27
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-03 23:29:56 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1080001
6 years, 6 months ago (2014-03-03 23:30:11 UTC) #29
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 6 months ago (2014-03-03 23:51:15 UTC) #30
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 00:31:22 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1100001
6 years, 6 months ago (2014-03-04 00:31:28 UTC) #32
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 03:45:07 UTC) #33
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 03:45:08 UTC) #34
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 03:49:38 UTC) #35
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 03:50:00 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1100001
6 years, 6 months ago (2014-03-04 03:50:10 UTC) #37
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 05:38:10 UTC) #38
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-04 05:38:20 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1100001
6 years, 6 months ago (2014-03-04 05:38:29 UTC) #40
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-03-04 10:22:46 UTC) #41
commit-bot: I haz the power
Retried try job too often on win_layout for step(s) webkit_lint http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout&number=25311
6 years, 6 months ago (2014-03-04 10:22:47 UTC) #42
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-05 00:46:47 UTC) #43
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rjwright@chromium.org/149363002/1120001
6 years, 6 months ago (2014-03-05 00:47:27 UTC) #44
rjwright
The CQ bit was unchecked by rjwright@chromium.org
6 years, 6 months ago (2014-03-05 03:01:13 UTC) #45
rjwright
The CQ bit was checked by rjwright@chromium.org
6 years, 6 months ago (2014-03-05 03:01:14 UTC) #46
commit-bot: I haz the power
6 years, 6 months ago (2014-03-05 17:54:13 UTC) #47
Message was sent while issue was closed.
Change committed as 168500

Powered by Google App Engine
This is Rietveld 408576698