Bug 179065

Summary: [Web Animations] Distinguish between an omitted and a null timeline argument to the Animation constructor
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, dbates, dino, esprehn+autocc, ews-watchlist, kangil.han, kondapallykalyan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=122912
Attachments:
Description Flags
Patch
none
Patch dino: review+

Antoine Quint
Reported 2017-10-31 12:11:18 PDT
The Web Animations spec says that the Animation constructor should distinguish between an omitted and a null timeline argument. In https://w3c.github.io/web-animations/#dom-animation-animation: "Run the procedure to set the timeline of an animation on animation passing timeline as the new timeline or, if a timeline argument is not provided, passing the default document timeline of the Document associated with the Window that is the current global object."
Attachments
Patch (18.00 KB, patch)
2018-01-26 11:35 PST, Antoine Quint
no flags
Patch (16.50 KB, patch)
2018-01-26 11:41 PST, Antoine Quint
dino: review+
Radar WebKit Bug Importer
Comment 1 2018-01-25 08:13:33 PST
Antoine Quint
Comment 2 2018-01-26 11:35:18 PST
Antoine Quint
Comment 3 2018-01-26 11:41:30 PST
Chris Dumez
Comment 4 2018-01-26 13:10:57 PST
Comment on attachment 332393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332393&action=review Custom constructor looks good but I have a couple of suggestions. > Source/WebCore/bindings/js/JSWebAnimationCustom.cpp:47 > + ASSERT(context->isDocument()); We prefer is<Document>(*context). > Source/WebCore/bindings/js/JSWebAnimationCustom.cpp:59 > + auto timeline = convert<IDLNullable<IDLInterface<AnimationTimeline>>>(state, state.argument(1), [](ExecState& state, ThrowScope& scope) { Could use uncheckedArgument(1) for performance. If there was no second argument, you would have return early.
Chris Dumez
Comment 5 2018-01-26 13:17:46 PST
Comment on attachment 332393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332393&action=review >> Source/WebCore/bindings/js/JSWebAnimationCustom.cpp:47 >> + ASSERT(context->isDocument()); > > We prefer is<Document>(*context). Actually, just drop this assertion, it is already inside the downcast<Document>() call below.
Antoine Quint
Comment 6 2018-01-27 01:26:25 PST
Note You need to log in before you can comment on or make changes to this bug.