1340445 - stylo: use DOM slots for EffectSet

Status: RESOLVED INVALID

(Core :: DOM: Animation, defect)

Description

[Boris Chiou [:boris]]

According to [1] and Bug 1340340 Comment 9, we should use DOM slots for EffectSet on stylo, instead of node properties.

[1] https://public.etherpad-mozilla.org/p/stylo-animation

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Note that AnimationCollection (which is used for CSS animations/transitions) also uses nsINode property.

Comment 2

[Boris Chiou [:boris]]

DOM slot: http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/dom/base/FragmentOrElement.h#245-253

Comment 3

[Bobby Holley (:bholley)]

FYI this may or may not be necessary, depending on the outcome of the discussion at in bug 1340340 comment 37.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

As far as I can tell, for CSS Animtions/Transitions we need this since an EffectSet is created during the traversal when we set animation style property, and the newly created EffectSet is immediately used for calculating animation styles.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> As far as I can tell, for CSS Animtions/Transitions we need this since an
> EffectSet is created during the traversal when we set animation style
> property, and the newly created EffectSet is immediately used for
> calculating animation styles.

Could we create it, but use a SequentialTask to attach it to the node? I don't know if it would be practical, but we should at least consider it.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #5)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> > As far as I can tell, for CSS Animtions/Transitions we need this since an
> > EffectSet is created during the traversal when we set animation style
> > property, and the newly created EffectSet is immediately used for
> > calculating animation styles.
> 
> Could we create it, but use a SequentialTask to attach it to the node? I
> don't know if it would be practical, but we should at least consider it.

Oh neat.  I am not sure how hard it is but I guess we can.

Comment 7

[Brian Birtles (:birtles)]

We decided not to take this approach but to rely on updating the data stored in node properties outside the parallel traversal in PreTraverse / SequentialTasks.