Summary: | animation-timing-function property with a list uses first item for all animations | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Marrin <cmarrin> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | dino, joybro201, simon.fraser, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Chris Marrin
2011-05-05 14:41:42 PDT
Created attachment 92474 [details]
test case
> -webkit-animation:
> bouncer 1s ease infinite alternate,
> mover 4s ease infinite alternate;
Hi, Chris
It seems that you made a mistake in the testcase. Shouldn't the two animations have different timing-functions?
Apart from the mistake, the bug you told is also reproduced on my environment.
Created attachment 98005 [details]
Patch
KeyframeAnimation::fetchIntervalEndpointsForProperty() is always getting a timingFunction from the first animation, even for the calculation of the second animation. This patch makes the function look for the correct animation by the keyframe's name. Comment on attachment 98005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98005&action=review > LayoutTests/animations/multiple-animations-on-an-element.html:35 > + runAnimationTest(expectedValues, null, null, true); Why does this have to disable the pause API? > Source/WebCore/page/animation/KeyframeAnimation.cpp:153 > + const Animation* matchedAnimation = getAnimationFromStyleByName(fromStyle, name()); > + if (matchedAnimation) We usually write this: if (const Animation* matchedAnimation = getAnimationFrom...) ... Created attachment 98168 [details]
Patch
(In reply to comment #5) > (From update of attachment 98005 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98005&action=review > > > LayoutTests/animations/multiple-animations-on-an-element.html:35 > > + runAnimationTest(expectedValues, null, null, true); > > Why does this have to disable the pause API? Because the test failed when the pause API is enabled, but I just found out that runAnimationTest always fails if the pause API is enabled and the test target animation has "infinite" iteration count. Is this a bug, too? Anyway, I changed the iteration count and enabled the pause API in the new patch. > > > Source/WebCore/page/animation/KeyframeAnimation.cpp:153 > > + const Animation* matchedAnimation = getAnimationFromStyleByName(fromStyle, name()); > > + if (matchedAnimation) > > We usually write this: > if (const Animation* matchedAnimation = getAnimationFrom...) > ... This is also changed. Thanks :) (In reply to comment #7) > (In reply to comment #5) > > (From update of attachment 98005 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=98005&action=review > > > > > LayoutTests/animations/multiple-animations-on-an-element.html:35 > > > + runAnimationTest(expectedValues, null, null, true); > > > > Why does this have to disable the pause API? > > Because the test failed when the pause API is enabled, but I just found out that runAnimationTest always fails if the pause API is enabled and the test target animation has "infinite" iteration count. > > Is this a bug, too? Probably. You should file a bug on it. > Anyway, I changed the iteration count and enabled the pause API in the new patch. Cool. Comment on attachment 98168 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98168&action=review > LayoutTests/ChangeLog:16 > + * animations/multiple-animations-on-an-element-expected.txt: Added. You should rename the test to multiple-animations-timing-function.html Created attachment 98183 [details]
Patch
(In reply to comment #9) > (From update of attachment 98168 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98168&action=review > > > LayoutTests/ChangeLog:16 > > + * animations/multiple-animations-on-an-element-expected.txt: Added. > > You should rename the test to multiple-animations-timing-function.html applied to the new patch. Comment on attachment 98183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98183&action=review > LayoutTests/ChangeLog:17 > + * animations/multiple-animations-on-an-element-expected.txt: Added. > + * animations/multiple-animations-on-an-element.html: Added. Still out of date. > Source/WebCore/ChangeLog:16 > + Test: animations/multiple-animations-on-an-element.html Out of date. Created attachment 98188 [details]
Patch
(In reply to comment #12) > (From update of attachment 98183 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98183&action=review > > > LayoutTests/ChangeLog:17 > > + * animations/multiple-animations-on-an-element-expected.txt: Added. > > + * animations/multiple-animations-on-an-element.html: Added. > > Still out of date. > > > Source/WebCore/ChangeLog:16 > > + Test: animations/multiple-animations-on-an-element.html > > Out of date. Oops.. I missed it! (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > (From update of attachment 98005 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=98005&action=review > > > > > > > LayoutTests/animations/multiple-animations-on-an-element.html:35 > > > > + runAnimationTest(expectedValues, null, null, true); > > > > > > Why does this have to disable the pause API? > > > > Because the test failed when the pause API is enabled, but I just found out that runAnimationTest always fails if the pause API is enabled and the test target animation has "infinite" iteration count. > > > > Is this a bug, too? > > Probably. You should file a bug on it. This is filed as Bug 63152. > > > Anyway, I changed the iteration count and enabled the pause API in the new patch. > > Cool. Comment on attachment 98188 [details] Patch Clearing flags on attachment: 98188 Committed r89462: <http://trac.webkit.org/changeset/89462> All reviewed patches have been landed. Closing bug. |