Bug 26943 – Transitions restart sometimes when they shouldn't

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Marrin]

This happens when they are two transitions, one of which has a slightly longer (50ms) duration than the other. The finish of the first one causes the second to run a second time.

I'm attaching a test case showing this.

[Chris Marrin]

Created attachment 32208 [details]
test case

[Chris Marrin]

Created attachment 32209 [details]
image used by test case

This image needs to be put into a resources subdirectory to be used with the test case

[Chris Marrin]

Created attachment 32210 [details]
Patch with test

NOTE: Please don't review this patch until we have done some tests with more complex content.

[Simon Fraser (smfr)]

Comment on attachment 32210 [details]
Patch with test

This breaks a testcase that I will attach.

[Simon Fraser (smfr)]

*** Bug 26689 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Created attachment 32248 [details]
Testcase that the patch breaks

[Chris Marrin]

The current patch file is in error. In CompositeAnimation.cpp, at line 200 there is a for loop:

        for (AnimationNameMap::const_iterator it = m_keyframeAnimations.begin(); it != kfend; ++it)

But the line that should be after it is missing. It should say:

        for (AnimationNameMap::const_iterator it = m_keyframeAnimations.begin(); it != kfend; ++it)
            it->second->setIndex(-1);

If you add that line, Simon's new test case works fine.

[Simon Fraser (smfr)]

Created attachment 32275 [details]
Testcase broken by the revised patch

[Simon Fraser (smfr)]

With the fixed-up patch, I see an assertion and test failing in the test I just attached. When you unhover the lower box, it doesn't start animating again.

[Simon Fraser (smfr)]

Fixed patch reviewed and committed:
http://trac.webkit.org/changeset/45554

[Simon Fraser (smfr)]

Bad news. I had to revert this because it broke LayoutTests/animations/transition-and-animation-1.html.

[Chris Marrin]

I have a fix for the transition-and-animation-1.html bug. The problem is that, if a single element has both a transition and a keyframe animation, the keyframe animation gets removed from the list before the transitions are updated. So when a keyframe animation ends, the transition gets updated and it looks like the style is different (because it is putting in the unanimated style from the old animation) without there being a keyframe animation. Updating transitions before animations fixes the problem. This makes sense since animations can override transitions, so running transitions first allows them to see if there are any animations overriding them.

I will submit a new patch soon...

[Chris Marrin]

Actually, I won't supply a new patch, since Simon as made an additional change. The fix is in CompositeAnimation.cpp. Take the updateKeyframeAnimations() method at line 263 and move it after the updateTransitions() method at line 266 (along with the comments, of course. If needed I can create a new patch with all my and Simon's fixes.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/45556