Open Bug 1419651 Opened 6 years ago Updated 2 years ago

Drop {Owning,NonOwning}AnimationTarget, use dom::Element directly

Categories

(Core :: DOM: Animation, enhancement, P3)

enhancement

Tracking

()

Tracking Status
firefox59 --- affected

People

(Reporter: hiro, Unassigned)

Details

I haven't yet audited what the barriers are to remove them though.  But it could happen.
How will you know if you are targetting a pseudo or not?
IsGeneratedContentContainerForBefore() or IsGeneratedContentContainerForAfter().
Are we already using the actual generated content element? Do we ever re-generate that? And do we really want to keep that alive?
(In reply to Brian Birtles (:birtles) from comment #3)
> Are we already using the actual generated content element? 

In servo side, we've been using the generated content for animations.

> Do we ever re-generate that?

I don't know.  What I know is that when content property is changed, reframing happens, but the generated element persists.  I don't know 'content: none' case though, I guess the generated element is destroyed.

> And do we really want to keep that alive?

I don't quite understand.  Keep alive for what?
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> (In reply to Brian Birtles (:birtles) from comment #3)
> > Are we already using the actual generated content element? 
> 
> In servo side, we've been using the generated content for animations.

We should update the comment in OwningAnimationTarget then (and probably a bunch of other places).

And I guess this bug needs to depend on removing the Gecko style system.

> > And do we really want to keep that alive?
> 
> I don't quite understand.  Keep alive for what?

I mean, after we set content: "", the Animation will still keep the generated content element alive because it holds a reference to it. Furthermore, it will be inspectable via script (to the extent the CSSPseudoElement interfaces permits). I'm just wondering if that's ok or if the generated content element will assume its dead.
(In reply to Brian Birtles (:birtles) from comment #5)
> > > And do we really want to keep that alive?
> > 
> > I don't quite understand.  Keep alive for what?
> 
> I mean, after we set content: "", the Animation will still keep the
> generated content element alive because it holds a reference to it.
> Furthermore, it will be inspectable via script (to the extent the
> CSSPseudoElement interfaces permits). I'm just wondering if that's ok or if
> the generated content element will assume its dead.

Ok (I guess you mean content: none?). A cumbersome case what I know is content property animation that changes none to others and vice versa.  In stylo the content animation does not work as expected (I don't recall gecko's behavior and I though daisuke already filed a bug for it, but couldn't find now), but if the animation holds the reference of the pseudo element, there is a chance that the content animation works fine.
Generated content gets unbound from the tree when reframed as of right now, and recreated. That is wrong and needs fixing for this bug to be doable.

After that, the element would be still alive and inspectionable, but it won't form part of the DOM tree (it will effectively be detached), which makes sense to me.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.