Bug 180830 - [Web Animations] Implement the cancel() method on Animation
Summary: [Web Animations] Implement the cancel() method on Animation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-14 12:59 PST by Antoine Quint
Modified: 2017-12-14 14:12 PST (History)
6 users (show)

See Also:


Attachments
Patch (10.63 KB, patch)
2017-12-14 13:02 PST, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-12-14 12:59:03 PST
We need to expose the cancel() method on the Animation interface.
Comment 1 Radar WebKit Bug Importer 2017-12-14 12:59:37 PST
<rdar://problem/36055816>
Comment 2 Antoine Quint 2017-12-14 13:02:20 PST
Created attachment 329387 [details]
Patch
Comment 3 Dean Jackson 2017-12-14 14:04:18 PST
Comment on attachment 329387 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329387&action=review

> Source/WebCore/animation/WebAnimation.cpp:342
> +        // 2. Reject the current finished promise with a DOMException named "AbortError".
> +        m_finishedPromise.reject(Exception { AbortError });

Are you confirming that all these changes are covered by existing tests? I don't see any changes in the expected results that are looking for this. If not, you should be writing new tests.

> Source/WebCore/animation/WebAnimation.cpp:356
> +        // 4. Create an AnimationPlaybackEvent, cancelEvent.
> +        // 5. Set cancelEvent's type attribute to cancel.
> +        // 6. Set cancelEvent's currentTime to null.
> +        // 7. Let timeline time be the current time of the timeline with which animation is associated. If animation is not associated with an
> +        //    active timeline, let timeline time be n unresolved time value.
> +        // 8. Set cancelEvent's timelineTime to timeline time. If timeline time is unresolved, set it to null.
> +        // 9. If animation has a document for timing, then append cancelEvent to its document for timing's pending animation event queue along
> +        //    with its target, animation. If animation is associated with an active timeline that defines a procedure to convert timeline times
> +        //    to origin-relative time, let the scheduled event time be the result of applying that procedure to timeline time. Otherwise, the
> +        //    scheduled event time is an unresolved time value.

Do you have bugs for all these missing steps?
Comment 4 Antoine Quint 2017-12-14 14:10:03 PST
(In reply to Dean Jackson from comment #3)
> Comment on attachment 329387 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329387&action=review
> 
> > Source/WebCore/animation/WebAnimation.cpp:342
> > +        // 2. Reject the current finished promise with a DOMException named "AbortError".
> > +        m_finishedPromise.reject(Exception { AbortError });
> 
> Are you confirming that all these changes are covered by existing tests? I
> don't see any changes in the expected results that are looking for this. If
> not, you should be writing new tests.
> 
> > Source/WebCore/animation/WebAnimation.cpp:356
> > +        // 4. Create an AnimationPlaybackEvent, cancelEvent.
> > +        // 5. Set cancelEvent's type attribute to cancel.
> > +        // 6. Set cancelEvent's currentTime to null.
> > +        // 7. Let timeline time be the current time of the timeline with which animation is associated. If animation is not associated with an
> > +        //    active timeline, let timeline time be n unresolved time value.
> > +        // 8. Set cancelEvent's timelineTime to timeline time. If timeline time is unresolved, set it to null.
> > +        // 9. If animation has a document for timing, then append cancelEvent to its document for timing's pending animation event queue along
> > +        //    with its target, animation. If animation is associated with an active timeline that defines a procedure to convert timeline times
> > +        //    to origin-relative time, let the scheduled event time be the result of applying that procedure to timeline time. Otherwise, the
> > +        //    scheduled event time is an unresolved time value.
> 
> Do you have bugs for all these missing steps?

They're all implemented via the single function call below those comments:

enqueueAnimationPlaybackEvent(eventNames().cancelEvent, std::nullopt, m_timeline ? m_timeline->currentTime() : std::nullopt);
Comment 5 Antoine Quint 2017-12-14 14:12:45 PST
Committed r225927: <https://trac.webkit.org/changeset/225927>