Summary: | REGRESSION (r187121): Multiple-keyframe animations not honouring ' forwards' fill-mode | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mario Sanchez Prada <mario> | ||||||||||
Component: | Animations | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cgarcia, commit-queue, cosimoc, dino, dstockwell, hyatt, mcatanzaro, mrobinson, ryanhaddad, simon.fraser, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | All | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=150326 | ||||||||||||
Attachments: |
|
Description
Mario Sanchez Prada
2015-10-19 06:23:33 PDT
I found that the patch for WebKit bug 146996 ("Safari mis-applies "animation-fill-mode: forwards" when using fractional iteration count") is the one who broke this behaviour: http://trac.webkit.org/changeset/187121 I've double checked it myself and I can confirm that reverting that patch gets the test case running fine once again. Simon, any idea? Created attachment 263896 [details]
Better testcase
Created attachment 263899 [details]
Patch
Comment on attachment 263899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263899&action=review The patch looks good to me, although someone more knowledgeable of this code should better review it. My only doubt is about the test case added, which I needed to modify slightly to be able reproduce the test locally, see my comments below... > Source/WebCore/ChangeLog:7 > + Nit. Extra trailing spaces in empty line. > LayoutTests/ChangeLog:5 > + Nit. Extra trailing spaces in empty line. > LayoutTests/ChangeLog:7 > + Ditto > LayoutTests/animations/fill-forwards-end-state.html:14 > + animation: anim1 1.5s 0.2s linear; I tested this layout test in my laptop now I can't see any difference with or without the patch, even though it DOES make a difference for my original test case and your better test case. After some fiddling, I found that making the animation a bit longer (2s instead of 1.5s) would make me reproduce the bug without your patch, so I was wondering if you could perhaps incorporate this change in the test? In any case, that's strange and perhaps exposes another bug, but I'm not 100% sure yet. In any case, this is what I did to be able to reproduce the problem using your layout test without the rest of the patch: - animation: anim1 1.5s 0.2s linear; + animation: anim1 2s 0.2s linear; > LayoutTests/animations/fill-forwards-end-state.html:28 > + ["anim1", 2, "box1", "left", 100, 2], This needs to be adjusted if you follow my suggestion on increasing the animation duration to something like this: - ["anim1", 2, "box1", "left", 100, 2], + ["anim1", 2.5, "box1", "left", 100, 2], Any duration > 1 should show the problem. Comment on attachment 263899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263899&action=review >> LayoutTests/animations/fill-forwards-end-state.html:14 >> + animation: anim1 1.5s 0.2s linear; > > I tested this layout test in my laptop now I can't see any difference with or without the patch, even though it DOES make a difference for my original test case and your better test case. > > After some fiddling, I found that making the animation a bit longer (2s instead of 1.5s) would make me reproduce the bug without your patch, so I was wondering if you could perhaps incorporate this change in the test? > > In any case, that's strange and perhaps exposes another bug, but I'm not 100% sure yet. In any case, this is what I did to be able to reproduce the problem using your layout test without the rest of the patch: > > - animation: anim1 1.5s 0.2s linear; > + animation: anim1 2s 0.2s linear; Sounds like a good suggestion. Comment on attachment 263899 [details]
Patch
Actually I'll fix the test as recommended.
animations/fill-forwards-end-state.html fails every time after r191502: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=animations%2Ffill-forwards-end-state.html The expected result just needs a number to be changed. Reopening to attach new patch. Created attachment 263949 [details]
Patch
Patch to update expected result.
Comment on attachment 263949 [details] Patch Clearing flags on attachment: 263949 Committed r191516: <http://trac.webkit.org/changeset/191516> All reviewed patches have been landed. Closing bug. |