[css3-animations] when animations interrupted by restyle, implicit 0% or 100% values (from base value) pick up value from animation

RESOLVED FIXED in mozilla9



8 years ago
8 years ago


(Reporter: maxaks, Assigned: bzbarsky)


Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)



(3 attachments, 1 obsolete attachment)



8 years ago
Posted file ffcss3anim.html
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110913 Firefox/9.0a1
Build ID: 20110913030846

Steps to reproduce:

I was trying to animate the border-color property between a base value (specified out of the @keyframes ruleset) and another value in a keyframe.

Actual results:

After a Repaint event, the starting value of the animated property is no longer the base value, but the value which was active before the Repaint event.

See the attached example:
Resizing the window triggers a Repaint and "blocks" the starting value of the animation.
For the first box, for example, the starting value becomes "black", and the animation therefore goes between "black" and "black".

Expected results:

Animation should continue properly, like it was just after the page load.

The specification does indeed say that you must include rules for at least the times 0% (or from) and 100% (or to), but doesn't indicate clearly if the starting value can not be defined elsewhere (as in, NOT in the keyframe rule).

This works in webkit.
Attachment #560138 - Attachment mime type: text/plain → text/html; charset=UTF-8
Confirming.  David, any idea what's up here?
Ever confirmed: true
Summary: CSS3 animations not rendered properly after repaint event → Restyling parent element stops CSS3 animations on children
If there were transitions involved, I would think that the problem is including the transition rules when reparenting or something.  But that shouldn't matter for animations, should it?
And in fact we explicitly skip transition rules when reparenting.  Maybe we need to do that for animations too?
If I do that, it seems to fix the bug.
So what's supposed to prevent this is that nsAnimationManager::CheckAnimationRule only calls BuildAnimations when !mPresContext->IsProcessingAnimationStyleChange().  (BuildAnimations is where we use the base value if there's nothing specified for 0% or 100%.)  I think the problem is that that's not sufficient for the cases where !mPresContext->IsProcessingRestyles().

Some untested work-in-progress is at:
but I'm pretty busy right now; may not have time to finish it for a bit.

(Boris, I'm curious if you think this makes sense -- my assumption is that with the async stuff we've done lately that reframes that result from processing restyles will actually lead to the new frames being created after mPresContext->IsProcessingRestyles() is no longer true -- but I could be wrong there, in which case this fix would actually be sufficient.)
OS: Windows 7 → All
Hardware: x86 → All
Summary: Restyling parent element stops CSS3 animations on children → [css3-animations] when animations interrupted by restyle, implicit 0% or 100% values (from base value) pick up value from animation
Version: 9 Branch → Trunk
(I wrote comment 5 before seeing comments 1 - 4, but submitted it anyway over the collision.)
OK, bz and I just discussed this; the reframing for restyles is sync, so bz's fix should be all that necessary.  But we should have a test that triggers both the reframing and non-reframing cases.
Posted patch test (obsolete) — Splinter Review
Without your patch, I get 1 failure:

failed | bug 686656 test 1 at 625ms - got 100px, expected 50px

and with your patch the test passes.
Attachment #561317 - Flags: review?(bzbarsky)
Comment on attachment 560286 [details] [diff] [review]
Make sure to skip animation rules as well as transition rules when we're reparenting a style context when not processing an animation restyle.

Attachment #560286 - Flags: review?(dbaron) → review+
Also, I'd like to add some assertions, but they fire too much; need to investigate:

Also, I realize the comment in the test is a bit misleading -- the 100% case doesn't actually matter since the animation has ended by the point we test.  I should probably add 'forwards' to the animation lines in both cases; then the 1000ms test should also fail without the patch.  I should check that...
Posted patch testSplinter Review
with -moz-animation-fill-mode: forwards
Attachment #561317 - Attachment is obsolete: true
Attachment #561317 - Flags: review?(bzbarsky)
Attachment #561319 - Flags: review?(bzbarsky)
Comment on attachment 561319 [details] [diff] [review]

Attachment #561319 - Flags: review?(bzbarsky) → review+
Assignee: nobody → bzbarsky
Flags: in-testsuite+
Priority: -- → P3
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.