Summary: | [Web Animations] REGRESSION: setting 'animation-name: none' after a 'fill: forwards' animation has completed does not revert to the unanimated style | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, cgarcia, dino, ews-watchlist, ryanhaddad, tsavell, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Safari Technology Preview | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=190307 | ||||||||
Attachments: |
|
Description
Antoine Quint
2018-10-03 12:06:40 PDT
Created attachment 351541 [details]
Patch
Comment on attachment 351541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351541&action=review > Source/WebCore/animation/AnimationTimeline.cpp:479 > ASSERT(animation); If this is always non-null, can the parameter be a Ref<>? > Source/WebCore/animation/DeclarativeAnimation.h:45 > + Element& target() const { return m_target; } Can this be const Element&? > LayoutTests/animations/animation-fill-forwards-removal.html:20 > + assert_equals(getComputedStyle(target).marginLeft, "100px", "The target element has style values from the final keyframe of its animation."); Have events always fired after we update the final style? I guess so. (In reply to Dean Jackson from comment #3) > Comment on attachment 351541 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351541&action=review > > > Source/WebCore/animation/AnimationTimeline.cpp:479 > > ASSERT(animation); > > If this is always non-null, can the parameter be a Ref<>? I'll look into it. > > Source/WebCore/animation/DeclarativeAnimation.h:45 > > + Element& target() const { return m_target; } > > Can this be const Element&? I'll look into it. > > LayoutTests/animations/animation-fill-forwards-removal.html:20 > > + assert_equals(getComputedStyle(target).marginLeft, "100px", "The target element has style values from the final keyframe of its animation."); > > Have events always fired after we update the final style? I guess so. Yes. Committed r236809: <https://trac.webkit.org/changeset/236809> Looks like after https://trac.webkit.org/changeset/236809/webkit imported/mozilla/css-transitions/test_event-dispatch.html has become a flakey timeout. Confirmed using command: run-webkit-tests --root testbuild-236809 imported/mozilla/css-transitions/test_event-dispatch.html --iterations 5000 -f running this on 236809 leads to a low number of timeout, running it on the previous built revision 236806 yields no timeouts. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fmozilla%2Fcss-transitions%2Ftest_event-dispatch.html Reopening to attach new patch. Created attachment 351723 [details]
Patch
(In reply to youenn fablet from comment #8) > Created attachment 351723 [details] > Patch Bad upload... |