Closed Bug 1344581 Opened 7 years ago Closed 7 years ago

stylo: Update CSS animations when @keyframes is changed

Categories

(Core :: CSS Parsing and Computation, defect, P2)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED DUPLICATE of bug 1364799

People

(Reporter: hiro, Unassigned)

References

(Blocks 1 open bug)

Details

This is a bug splitting off from bug 1341985.
In bug 1341985 we are going to create a SequentialTask to update CSS animations when animation properties are changed. But in that bug we don't check @keyframes rules at all.

test_animations_dynamic_changes.html is a test case for this.
(This is probably completely unrelated, but I'm pretty sure we don't notify animation mutation observers when keyframes are changed either.)
Oh dear! Should we notify in DoUpdateProperties? The keyframe values on devtools should be updated immediately? I am not sure.
Actually, never mind. I think I'm wrong. The other day when I was looking at the code in UpdateOldAnimationPropertiesWithNew[1] I noticed we compare the timing properties to see if they have changed but we don't compare the keyframes. Looking at it more closely I'm pretty sure SetKeyframes does the notification for us (and I think I had a TODO item somewhere about doing the same thing for timing properties).

[1] http://searchfox.org/mozilla-central/rev/e844f7b79d6c14f45478dc9ea1d7f7f82f94fba6/layout/style/nsAnimationManager.cpp#370
Oops, I did want to block bug 1288269 actually.

(In reply to Brian Birtles (:birtles) from comment #3)
> Actually, never mind. I think I'm wrong. The other day when I was looking at
> the code in UpdateOldAnimationPropertiesWithNew[1] I noticed we compare the
> timing properties to see if they have changed but we don't compare the
> keyframes. Looking at it more closely I'm pretty sure SetKeyframes does the
> notification for us (and I think I had a TODO item somewhere about doing the
> same thing for timing properties).
> 
> [1]
> http://searchfox.org/mozilla-central/rev/
> e844f7b79d6c14f45478dc9ea1d7f7f82f94fba6/layout/style/nsAnimationManager.
> cpp#370

You did it! :-)
Blocks: stylo-css-animations
No longer blocks: 1341985
Priority: -- → P2
This will be covered by bug 1364799.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.