Bug 22773 - pauseAnimationAtTimeOnElementWithId() does not work with animations using multiple keyframes
Summary: pauseAnimationAtTimeOnElementWithId() does not work with animations using mul...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P3 Normal
Assignee: Pierre-Olivier Latour
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-09 17:42 PST by Pierre-Olivier Latour
Modified: 2008-12-10 10:01 PST (History)
3 users (show)

See Also:


Attachments
Patch (44.80 KB, patch)
2008-12-09 18:32 PST, Pierre-Olivier Latour
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Olivier Latour 2008-12-09 17:42:43 PST
pauseAnimationAtTimeOnElementWithId() does not work with animations using multiple keyframes
Comment 1 Pierre-Olivier Latour 2008-12-09 18:32:37 PST
Created attachment 25909 [details]
Patch

The issue is that KeyframeAnimation::animate() does not compute the ellapsed time exactly the same way that AnimationBase::progress() does.
Comment 2 Simon Fraser (smfr) 2008-12-09 21:18:02 PST
I think the testcase should be a text test (dumpAsText()), and should put "PASS" or "FAIL" in the document rather than throwing.
Comment 3 Darin Adler 2008-12-10 09:41:19 PST
Comment on attachment 25909 [details]
Patch

Could we change this so that KeyframeAnimation::animate() shared more code with AnimationBase::progress() so they can't get out of sync?

r=me as-is
Comment 4 Pierre-Olivier Latour 2008-12-10 09:42:21 PST
> I think the testcase should be a text test (dumpAsText()), and should put
> "PASS" or "FAIL" in the document rather than throwing.

If this test fails, the render tree output will be different.

Throwing just happens if the animation does not exist in the first place, which is not what this test looks at. It's also consistent with the other DRT animation API tests added so far.
Comment 5 Pierre-Olivier Latour 2008-12-10 09:42:57 PST
(In reply to comment #3)
> (From update of attachment 25909 [details] [review])
> Could we change this so that KeyframeAnimation::animate() shared more code with
> AnimationBase::progress() so they can't get out of sync?

Absolutely, we have a radar on this, Chris will look at it.
Comment 6 Pierre-Olivier Latour 2008-12-10 10:01:08 PST
Sending        LayoutTests/ChangeLog
Adding         LayoutTests/animations/animation-drt-api-multiple-keyframes.html
Adding         LayoutTests/platform/mac/animations/animation-drt-api-multiple-keyframes-expected.checksum
Adding  (bin)  LayoutTests/platform/mac/animations/animation-drt-api-multiple-keyframes-expected.png
Adding         LayoutTests/platform/mac/animations/animation-drt-api-multiple-keyframes-expected.txt
Sending        LayoutTests/platform/win/Skipped
Sending        WebCore/ChangeLog
Sending        WebCore/page/animation/KeyframeAnimation.cpp
Transmitting file data ........
Committed revision 39176.