[web-animations-1] Alternative to FillAnimation: replace events

[birtles]

In #3210 and #2054 I have been working on a proposal to allow filling animations to not leak memory. I have prepared spec text for that and implemented (but not yet landed) it in Firefox but it is quite complex. This is an alternative approach that might be simpler.

Summary

Proposal: automatically cancel any completely replaced filling script animations and fire a 'replace' event that includes their animated style so the author can manually set that on the element if they wish to preserve the animation's effect:

e.g. instead of

left.addEventListener('click', () => {
  target.animate(
    { transform: 'translateX(-80px)', composite: 'add' },
    { duration: 1000, easing: 'ease', fill: 'forwards' }
  );
});

(Live example)

The author would be required to write:

left.addEventListener('click', () => {
  const animation = target.animate(
    { transform: 'translateX(-80px)', composite: 'add' },
    { duration: 1000, easing: 'ease', fill: 'forwards' }
  );
  animation.onreplace = evt => {
    target.style.transform = evt.computedStyle.transform;
  };
});

When additive animations are not in use, the authors would typically be free to ignore the event unless they planned to cancel the overriding animation.

Proposal

Define a forever-filling animation as:

• The existence of the animation is not prescribed by markup.
  That is, it is not a CSS animation with an owning element, nor a CSS transition with an owning element.
• The animation's play state is finished.
• The animation is associated with a monotonically increasing timeline.
• The animation has an associated target effect.
• The target effect associated with the animation is in effect (i.e. it has the appropriate fill mode).

Any time a forever-filling animation finishes, run the following steps:

1. Look up the effect stack for the target element of the animation that finished.
   (Once we introduce GroupEffects, this will involve visiting all target elements.)

2. Let replaced animations be an empty set.

3. Proceeding from the bottom of the effect stack[1], for each forever-filling animation, animation, where all the target properties (once resolved to physical longhand properties) are also specified by one or more forever-filling animations that have a greater composite order:

   1. Calculate the computed value of each of animation's target effect's target properties at its current position in the effect stack.

   2. Add (animation, computed values) to replaced animations.

4. For each pair (animation, computed values) in replaced animations:

   1. Cancel animation (but don't dispatch a 'cancel' event? See below).

   2. Create an AnimationReplaceEvent, replaceEvent.

      (I'm not sure if we should re-use AnimationPlaybackEvent or not here.)

   3. Set replaceEvent’s type attribute to 'replace'.

   4. Set replaceEvent’s computedStyle attribute to computed values.

      (For implementations that implement CSS Typed OM, there is also
      a computedStyleMap attribute.)

   5. Set replaceEvent’s timelineTime attribute to the current time of the timeline with which animation is associated. If animation is not associated with a timeline, or the timeline is inactive, let timelineTime be null.

   6. If animation has a document for timing, then append replaceEvent to its document for timing's pending animation event queue along with its target, animation. For the scheduled event time, use the result of converting the most recently finished overriding animation’s target effect end to an origin-relative time.

      Otherwise, queue a task to dispatch replaceEvent at animation. The task source for this task is the DOM manipulation task source.

[1] Obviously no one would actually implement this as proceeding from the bottom of the stack. You'd almost certainly want to proceed from the top and go down. The point here is just to ensure that the replaced animations and hence the replace events are dispatched in order from lower in stack to higher.

Since replace events are dispatched in composite order, script can blindly just set the computed style on the target element and wait for subsequent events to override it.

Given that replace events are typically queued as part of updating timing and run before updating the rendering, there should be no visual glitch as a result of updating style in this manner.

In order to generate the replace events an additional style flush will typically be required to compute the correct computed style. Implementations might want to avoid doing this when there are no replace event listeners registered. Such an optimization could possibly produce Web-observable changes with regards to dispatching transitions so the behavior with regard to style change events might need to be specified.

IDL

partial interface Animation {
    attribute EventHandler onreplace;
}

[Exposed=Window,
 Constructor (DOMString type, optional AnimationReplaceEvent eventInitDict)]
interface AnimationReplaceEvent : Event {
    readonly attribute double? timelineTime;
    readonly attribute CSSStyleDeclaration computedStyle;
    readonly attribute StylePropertyMapReadOnly computedStyleMap;
};
dictionary AnimationReplaceEventInit : EventInit {
    double? timelineTime = null;
};

Relationship to finish events

Finish events sort before replace events. It is the finishing of a higher-priority animation that triggers the replacing.

For an animation that is replaced as soon as it finishes, the finish event still fires first since logically it finishes before it is replaced.

Relationship to the finished Promise

Since finishing happens before replacing, the finished promise will be resolved and hence will not be rejected when an animation is subsequently canceled by being replaced.

Relationship to cancel events

I suspect cancel events should not be fired even though we effectively cancel these animations. Dispatching cancel events will likely confuse authors and might break existing content. Rather, we should consider replacing a special kind of canceling; neutering perhaps.

Side effects

Replace events give the author the computed animated style for the effect that is being canceled so that they can apply it directly. However, since the returned value will be comprised of computed values for longhand (and probably physical-only) properties, any context-sensitive values (em units, CSS variables(?) etc.) will be lost. Furthermore, since CSS cannot yet represent additive values in static contexts, any additive behavior will also be lost.

These side effects should at least be somewhat apparent from the naming (computedStyle) and steps taken to achieve the effect.

Compatibility

Currently only effects that fully replace the underlying value are shipped so the only situation where this proposal would produce an observable change is if an overriding forever-filling animation is canceled while there is an underlying forever-filling animation. Under this proposal, the underlying animation would have been canceled so the result would differ.

I don't expect this is common but usage of the cancel() method is surprisingly high (3% of page loads!)

Issues

Ideally it would be good to offer a way for authors to opt-out of the replacing behavior.

For example, if, in the replace event callback, the author could re-instate the animation to its filling state, they could avoid the side effects mentioned above. This might produce a situation that effectively leaks memory, but it would be at the author's explicit direction much as if they created new Elements ad infinitum (and unlike the case where they fire off filling animations without necessarily being aware the animations will never disappear).

I don't have any idea for what a suitable API for reinstating such an Animation would be, however.

[birtles]

/cc @graouts @flackr @stephenmcgruer This proposal sounds similar to a couple of things you have mentioned in the past. Let me know what you think.

[birtles]

Thinking about this a bit further, automatically canceling is a bit weird. Under this proposal two animations might start at the same time but one ends up in the "finished" state while another ends up in the "idle" state just because of a difference in fill mode. Fill mode and play state should be orthogonal.

Perhaps we need some orthogonal bit of state for reflecting replaced-ness. e.g.

enum ReplaceState {
  no, // Not yet replaced
  replaced,
  persisted
}

partial interface Animation {
  attribute EventHandler onreplace;
  readonly attribute ReplaceState replaced;
  void persist(); // Throws if replaced === 'no'?
}

(I also looked at putting this onto the effect instead so that in a GroupEffect-world we could replace on an effect-by-effect basis but I think it complicates things a lot and authors will not want that fine-grained behavior. It's also at odds with the model in a number of ways--putting event handlers on effects, making effects stative, etc.)

[flackr]

Could you clarify, in the summary you state that only completely "replaced" animations are canceled, but in step 3 of the proposal you state that an animation is replaced if all of its properties are specified by any subsequent fill: forwards animation, without mention that they need to be composite: replace. Was this intentional or did you mean to require composite replace?

It seems like if we limit this to composite: replace we don't need to compute the style and pass it with the event as a later animation has replaced it. I suspect this will greatly simplify the implementation. The developer can save a list of the replaced animations. Perhaps the replaced animations could even maintain their start time such that if you reattach them in the future they start in the finished fill: forwards state?

[birtles]

Could you clarify, in the summary you state that only completely "replaced" animations are canceled, but in step 3 of the proposal you state that an animation is replaced if all of its properties are specified by any subsequent fill: forwards animation, without mention that they need to be composite: replace. Was this intentional or did you mean to require composite replace?

No it doesn't require composite: replace.

It seems like if we limit this to composite: replace we don't need to compute the style and pass it with the event as a later animation has replaced it.

I think that wouldn't help with the example though. In that example, we would continue to leak memory.

[flackr]

Thanks, this makes sense now.

I worry about requiring developer cooperation with the animation system when the animation's effects are replaced. It sounds like forgetting to do this could go unnoticed and result in occasional unpredictable behavior. If the developer does know to handle this, they feasibly could also handle applying their animation's final output to the inline style on animationend and canceling the animations. The missing piece (I think) would be how to compute the result of adding the final keyframes' styles, which may not be trivial.

It would be nice to have more predictable behavior that "just works", whether that's replacing completed fill: forwards additive animation with a single generated fill: forwards animation having the same effect or something more noticeable like modifying the topmost animation or automatically applying the final value to the inline style.

[birtles]

Refining this further, I have two particular questions that might help clarify how this should behave.

One stems from something @danwilson mentioned on the Animation at Work slack (#waapi channel), and one is based on @flackr's question above.

1) Do we replace even if there are underlying animations?

e.g.

Anim C ← Top of stack
Anim B ← Filling
Anim A ← Still playing

(Assume all animations target the same set of properties on the same element and all are additive.)

Do we replace Anim B?

If we say "Yes" → whether or not the replace event callback sets specified style using the styles from the replace event, the end result will be:

Anim C
Anim A
[ Specified style ]

which is clearly different. In this case script would need to know that there is an underlying effect so it can call persist(). We could expose that information in the event but that feels a bit awkward.

If we say "No" → Then script can naively just clobber specified style with whatever is passed in and generally the result will not change since, once Anim A ends, we'll end up dispatching a replace event on Anim A then Anim B.

That's simpler but it will mean if you have a never-ending underlying animation, we'd never dispatch any replace events for the remainder of the stack (unless we extend replace events to cover unfinished animations too). In effect, there would be cases that can still leak.

2) Do we replace only once all properties are covered?

I initially assumed so but the following case has me concerned:

Anim B ← Animates `left`. Additive. Still playing.
Anim A ← Animates `left` and `top`. Finished and filling.

So far, so good. Anim A is not replaced since not all of its properties are replaced yet. Later, however, we trigger Anim C as follows:

Anim C ← Animates `top`. Still playing.
Anim B ← Animates `left`. Additive. Still playing.
Anim A ← Animates `left` and `top`. Finished and filling.

Anim A is now replaced so we drop it. If script doesn't handle the replace event and either set the specified style or call persist(), the output on the left property will change sharply since we will have just:

Anim C ← Animates `top`.
Anim B ← Animates `left`. Additive.

That's probably ok, but it might be a bit surprising that the output changes suddenly due to two unrelated animations.

An alternative might be to replace on a property-by-property basis but I think that would leak to a much more complex API and it's probably ok to expect authors to call persist() when they're dealing with complex layering of additive animations like this.

[birtles]

The missing piece (I think) would be how to compute the result of adding the final keyframes' styles, which may not be trivial.

Right, especially if the animation is filling in between two keyframes.

We may want to add an API for querying intermediate animation values in future anyway, but I think in this context the advantage of putting it on the events is that we snapshot the values.

That's particularly important because the Animation should already be replaced by the time the replace event is processed (so it would be too late to query animation values without un-replacing the animation first). Snapshotting also makes this work correctly when we have multiple replace events (since clobbering specified style in one event callback won't affect the snapshotted value passed to the next event callback).

It would be nice to have more predictable behavior that "just works", whether that's replacing completed fill: forwards additive animation with a single generated fill: forwards animation

I think that's essentially the FillAnimation proposal (but with coalescing of these generated animations, since otherwise we'd continue to leak).

something more noticeable like modifying the topmost animation or automatically applying the final value to the inline style.

I'm a little averse to this because I'm afraid it will be surprising to the author if we silently update specified style or their Animation objects. I'm also afraid it will lead to awkward race conditions--more so than if we require authors to actually make the change themselves.

(I believe we actually tried to implement updating specified style in a previous iteration of this discussion and it didn't work but I don't recall what the specific issue was.)

[birtles]

I'm going to try and document the alternative proposal we discussed at the meeting yesterday, but first, one thing I recalled after our meeting was that even if we change the order of operation for the effect stack so that interpolation happens before addition, I don't think it will drastically simplify implementation or improve performance of any FillAnimation-like solution.

Consider lengths and percentages. For some properties it is possible to interpolate from 5em to 20%, for example. The promise of FillAnimation is that it makes the substitution unobservable (at least from the point of view of the output style). Therefore it needs to preserve the context-sensitive behavior of these units.

The simplest way to do that is simply to preserve the interpolation interval endpoint(s) (there will be two endpoints if the animation fills mid-interval, one otherwise). For a stack using addition, this simple approach involves preserving the interval endpoint(s) for the whole additive part of the stack.

If that proves undesirable, a UA can introduce various optimizations. For example, in the case where the units are all the same, e.g. all em units, the UA could add them together and collapse that part of the stack.

If the UA wants to optimize further still, it could perhaps build up buckets for each unit type and add appropriate portions to each bucket. The final values of each bucket could be stuck in a single calc() expression that would be suitably responsive.

Implementing the above example optimizations is quite involved and the end result still does not account for variables and only applies to length/percentages.

My experience working on FillAnimations for several months is that it will always be an incomplete solution. UAs can optimize for different kinds of content but the amount of complexity involved rises quickly. As a result, the implementation I wrote does not collapse additive stacks--it retains only the relevant endpoint(s) in a much more compact structure so that magnitude of the memory leak is reduced, but it still effectively leaks.

[birtles]

Yesterday at our sync up we discussed yet another alternative approach which is somewhat of a middle-ground between this proposal and the FillAnimation proposal (#3210).

The idea is something like the following:

• As with the proposal in this issue, once indefinitely filling animations are completely covered by other indefinitely filling animations, they are "replaced" (tentative name).

  (As with the proposal in this issue, this basically means canceling the animation but I don't think we want to actually cancel the animation for reasons described earlier in this issue. Instead we probably want some orthogonal state to indicate that the animation is no longer effective.)

• However, unlike this proposal, at the point when an animation is "replaced", a new FillAnimation-like object is automatically generated. That is, a read-only Animation that preserves the fill value in all its fidelity (e.g. responding to font-size and variable changes).

• The new FillAnimation occupies the same position in the global animation list as the replaced Animation.

• When a newly-generated FillAnimation includes the effect of an existing FillAnimation, the previous FillAnimation is also "replaced".

• A replaced Animation is basically useless. It's not clear if we want to be able restore it or not.
  (Supposing we did have a persist() / 'restore()' method on the original Animation, it would likely need to cancel the generated FillAnimation. But what if the generated FillAnimation had itself been replaced?)

• Calling getAnimations() returns said FillAnimation objects (excluding ones that have been since replaced).

That's a fairly high-level overview. I haven't thought through the specifics of it yet.

Advantages:

• Compared to the proposal in this issue, this alternative proposal ensures the style output is unaffected, even when using additive animation. The author doesn't need to do anything.

• Compared to the FillAnimation proposal, it avoids the complexity introduced by making existing Animation objects remain functional.

Disadvantages:

• It introduces two new concepts for authors to understand: "replacing" and FillAnimations. That said, many authors will never need to deal with either.

• It suffers the same problems as FillAnimation with regards to it being difficult to compact additive content. It will always be an incomplete solution.

• It has the same compatibility uncertainly as the proposal in this issue.

• It's a bit more magical than the proposal in this issue.

Personally, my main concern is that I'm still not sure that this alternative offers a net reduction in complexity. Although it reduces complexity surrounding existing Animation objects, it still includes many of the harder parts of the FillAnimation proposal (e.g. generating a sensible result from getKeyframes() for some internal representation of an additive stack) and adds to it the complexity of the replacing functionality.

If it turns out that this proposal is comparable to the FillAnimation one in terms of implementation complexity, then I would lean towards the original FillAnimation proposal for its conceptual simplicity.

[birtles]

Worth mentioning on this topic is the removedOnCompletion flag in Core Animation.

It's basically equivalent to the fill: 'forwards' so it's not a solution in and of itself, but we might be able to re-use this terminology to produce something that is familiar to developers using CA. e.g. instead of "replace" events, "remove" events might be more familiar, not to mention more accurate.

[stephenmcgruer]

I'm not clear what the relevance of removedOnCompletion is; as you said, it's approximately equivalent to using (or not using) fill: 'forwards' (it defaults to YES, so their default is the same as Web Animations - no fill - right?).

On the interpolation topic, I believe @flackr was working on an idea that hopefully addresses the "doesn't simplify things" concern.

[birtles]

I'm not clear what the relevance of removedOnCompletion is; as you said, it's approximately equivalent to using (or not using) fill: 'forwards' (it defaults to YES, so their default is the same as Web Animations - no fill - right?).

Yeah, like I said, it's just terminology.

On the interpolation topic, I believe @flackr was working on an idea that hopefully addresses the "doesn't simplify things" concern.

Great. I'd love to hear it as soon as possible.

[graouts]

Sorry for taking so long to review this.

Just to make sure I don't misunderstand, this proposal suggests that an animationA that gets entirely superseded by an animationB is completely discarded and the author should commit the style if he wants to keep track of it?

Are this proposal and the FillAnimation proposal exclusive? Couldn't we have FillAnimation for animations that aren’t completely replaced?

[birtles]

Yes that's right. I'd prefer to either do this proposal or the FillAnimation proposal. FillAnimation is sufficiently complex on its own (for both authors and implementers) so I wouldn't want to complicate it further.

[graouts]

I wonder if we could have API directly to commit an animation. I worry that a lot of authors would write something like this:

function commitStyles(replaceEvent)
{
    const animation = replaceEvent.target;
    const element = animation.effect.target;
    for (let property of Object.getOwnPropertyNames(animation.getKeyframes())) {
        if (property == "offset" || property == "computedOffset" || property == "easing" || property == "composite")
            continue;
        element.style[property] = replaceEvent.computedStyle[property];
    }
}

target.animate(…).onreplace = commitStyles;

I think it'd be awesome if we could provide some built-in function on an effect to apply its animated values directly to the target, with potentially a list of properties to commit.

// Commit all animated properties to inline style upon completion.
target.animate(…).onreplace = evt => {
    evt.target.effect.commitAnimatedProperties();
};

// Commit select animated properties to inline style upon completion.
target.animate(…).onreplace = evt => {
    evt.target.effect.commitAnimatedProperties("transform", "opacity");
};

Naming certainly could improve, there likely is a catchier and shorter name for the function of committing some animated properties to inline style.

We could take it one step further and forgo the use of the replace event entirely with:

// Commit all animated properties to inline style upon completion.
target.animate(
    { transform: 'translateX(-80px)', composite: 'add' },
    { duration: 1000, easing: 'ease', commit: 'all' }
);

// Commit select animated properties to inline style upon completion.
target.animate(
    { transform: 'translateX(-80px)', composite: 'add' },
    { duration: 1000, easing: 'ease', commit: ['transform', 'opacity'] }
  );

[graouts]

I also suggest we add informative text that explains that this fill: forwards has serious implications for performance and that an alternative approach should be considered when creating scripted animations.

[flackr]

Disadvantages:

• It introduces two new concepts for authors to understand: "replacing" and FillAnimations. That said, many authors will never need to deal with either.
• It suffers the same problems as FillAnimation with regards to it being difficult to compact additive content. It will always be an incomplete solution.

If we can switch the composition and interpolation order as proposed in #2204 I believe this will be a complete solution as we can always replace with the equivalent transform matrix.

• It has the same compatibility uncertainly as the proposal in this issue.

I think the compatibility risk is reduced over this proposal. The only time existing sites would break is if they tried to use completed animations, whereas in this proposal animations will silently be dropped and change output value.

• It's a bit more magical than the proposal in this issue.

I disagree, I think that this proposal describes a hard to predict set of circumstances for dropping animations where an always replace model has very predictable behavior. We could however make it only replace at the point in time where there was more than one filling animation.

Personally, my main concern is that I'm still not sure that this alternative offers a net reduction in complexity. Although it reduces complexity surrounding existing Animation objects, it still includes many of the harder parts of the FillAnimation proposal (e.g. generating a sensible result from getKeyframes() for some internal representation of an additive stack) and adds to it the complexity of the replacing functionality.

I believe that resolving #2204 would mean that the keyframes of a replaced animation could simply have the transform matrix which greatly reduces the complexity.

If it turns out that this proposal is comparable to the FillAnimation one in terms of implementation complexity, then I would lean towards the original FillAnimation proposal for its conceptual simplicity.

Agreed

[birtles]

I think it'd be awesome if we could provide some built-in function on an effect to apply its animated values directly to the target, with potentially a list of properties to commit.

I definitely agree we want to make this easier. A few comments:

• It doesn't make it clear that we're committing the computed style. And I think we want to commit the computed style since the fill value could be the result of interpolating 70% between 50em and 200px and we don't have a way of representing that as a specified value yet (although in future perhaps we ought to have interpolate(50em, 200px, 70%)?). We could probably just fix that with naming, e.g. commitComputedStyle.

• It might not scale well to group effects. Which effect do you call it on? Do you have to iterate over all of them? We could probably fix that by just moving this method to Animation -- that would also take care of iterating over all the effects and committing their styles to the corresponding target elements. If we need more fine-grained controls, we could add it to KeyframeEffect as well later.

[birtles]

Disadvantages:

• It introduces two new concepts for authors to understand: "replacing" and FillAnimations. That said, many authors will never need to deal with either.
• It suffers the same problems as FillAnimation with regards to it being difficult to compact additive content. It will always be an incomplete solution.

If we can switch the composition and interpolation order as proposed in #2204 I believe this will be a complete solution as we can always replace with the equivalent transform matrix.

I'm not sure it does since the problem there is not just combining matrices, but also dealing with context-sensitive values.

[birtles]

Incorporating some of the suggestions that have come up on this thread, the proposed IDL might be something like:

enum RemoveState {
  "no", // Not yet removed
  "removed",
  "persisted"
};

partial interface Animation {
  attribute EventHandler onremove;
  readonly attribute RemoveState removed;

  void commitComputedStyles(optional sequence<DOMString> propertiesToCommit);
  void persist();
};

[Exposed=Window,
 Constructor (DOMString type, optional AnimationRemoveEvent eventInitDict)]
interface AnimationRemoveEvent : Event {
    readonly attribute double? timelineTime;
    readonly attribute CSSStyleDeclaration computedStyle;
    readonly attribute StylePropertyMapReadOnly computedStyleMap;
};

dictionary AnimationRemoveEventInit : EventInit {
  double? timelineTime = null;
  CSSStyleDeclaration computedStyle;
  StylePropertyMapReadOnly computedStyleMap;
};

Or, alternatively:

enum CommitKeyword = { "all", "none" };

partial dictionary KeyframeEffectOptions {
  commit: CommitKeyword or sequence<string> = "none";
};

partial interface KeyframeEffect {
  readonly attribute (CommitKeyword or FrozenArray<string>) commit;
};

Unfortunately, I don't think the latter alone covers the case where the author wants to persist the full effect so I suspect we would need most of the first part too (but probably minus the commitComputedStyles part).

[birtles]

Discussed this with @stephenmcgruer @flackr @graouts @majido and Olga yesterday. Summarizing some of the points:

• Changing the order of interpolation vs composition may be desirable but it does not solve the issue of filling animations (specifically it does not handle the need to retain animations with context-sensitive values such as those that use CSS variables, em, rem, vh, % units etc.) so should probably be discussed as a separate issue.
• Concern about the declarative version of commit -- it hides the fact that this is simply writing computed values to specified style. As a result it's not obvious that this loses information (e.g. variables, em units etc. get lost) or that it will change the layering (e.g. other animations may start overriding the result since they are higher in the cascade). Having the author apply the values to specified style makes it more obvious and makes it their responsibility.
• Long-term we probably want some other mechanism for triggering transitions from script and it's tempting to try to cover that here, but maybe this is not the right way. e.g. a proper solution might keep the result in a separate animation layer of the cascade or have some way to make that distinction observable.
• If we were to go ahead with the declarative commit behavior, it should probably apply at the point when the animation finishes, as opposed to when it is replaced. (And this raises questions about its relationship to fill as well as to any persist()-like functionality.)
• The above points lead us towards taking a more procedural approach to committing values for now and leaving open the door to a better way of generating transition-like animations in future.
  • (Perhaps after introducing an interpolate() function to CSS along with an addition mechanism such as !add--doing that would allow us to more faithfully retain the result of animations.)
• Ideally, the procedural approach would be an extension to CSSStyleDeclaration--i.e. something generally useful that makes the loss of context-sensitivity / cascade position obvious. However, this will not scale well to GroupEffects.

That last point highlights a problem with the proposal in this issue: specifically the proposed replace/remove event includes a single computedStyle member but in future we may have Animations that target multiple elements, and hence require multiple sets of computedStyle.

There are a few possibilities for addressing this last point:

1. Proceed with extending CSSStyleDeclaration and add a computedStyle member to KeyframeEffect. Script would then need to write:

   animation.onremove = evt => {
     animation.effect.target.style.merge(animation.effect.computedStyle);
   };

   (Presumably we could make merge() to take an optional list of properties to merge.)

   and in a GroupEffect world:

   animation.onremove = evt => {
     applyComputedStyle(animation.effect);
   };
   
   function applyComputedStyle(effect) {
     if (effect.children) {
       effect.children.forEach(applyComputedStyle);
     } else if (effect instanceof KeyframeEffect) {
       effect.target.style.merge(effect.computedStyle);
     }
   }

   This is pretty horrible, but at least the author is in full control of what happens and can see what is happening -- there's no magic.

2. Add a method to Animation to do this

   animation.onremove = evt => {
      animation.commitStyles();
   };

   This is less obvious -- it's not clear that it's the computed style that gets applied (although "commit" does suggest something final to it) nor that it changes the composite order (although "Styles" hopefully suggests the "style" attribute, i.e. specified style). commitResult would address the first concern but probably obscure the latter.

   On the other hand it makes supporting GroupEffect much simpler--code continues to work without any changes. Furthermore, it exactly parallels the approach when persisting.

   Presumably commitStyles would take an optional array of longhand physical property names to clobber.

[birtles]

Presumably commitStyles would take an optional array of longhand physical property names to clobber.

Actually, I think it would have to accept shorthand (and probably logical) properties too. Not only is that easier for authors but it also means that if any longhand gets promoted to a shorthand (as they tend to do) code keeps working.

[birtles]

I've implemented this alternative proposal in Firefox including pretty thoroughgoing web-platform-tests. I plan to prepare a spec PR tomorrow.

The IDL as it currently stands looks like:

enum AnimationReplaceState {
  "ok",
  "removed",
  "persisted"
};

partial interface Animation {
  readonly attribute AnimationReplaceState replaceState;
  attribute EventHandler onreplace;

  void persist ();
  void commitStyles (optional sequence<DOMString> propertiesToCommit);
};

Most of it is quite straight forward. The key part is that the check for replacement happens after updating the time of all timelines but before running any animation event handlers or promise callbacks (that is it happens as part of the update animations and send events procedure).

That seems to work well, making the replacement predictable and producing a consistent state when script runs.

I've made it possible to call persist() even when the replaceState is ok and even when the animation is not finished or filling. Most of the reasons I thought of for not doing that didn't hold up when I looked into it and being able to call persist() in advance actually proves to be useful when writing some of the tests.

I've also made it possible to call commitStyles() at any point in an animation's playback for largely similar reasons but also since it seems useful to decouple it from the replacing behavior so authors can manually commit and then cancel animations as needed.

Most of the outstanding questions concern the commitStyles function. They include:

1. What do we call it?

• commitStyles?
• commitComputedStyles?
• commitResult?
• flatten?

I've gone with commitStyles for now although I start to like commitResult too. Happy to bikeshed as needed.

2. Should commitXXX cause the animation to be removed?

At first it seems like it should because doing so:

• makes the removed state more useful, and
• gives the author more control over removal -- rather than waiting for auto-removal they can trigger it manually

However, I think it should not because:

• It doesn't make sense for the replaceState member to change based on calling commitXXX.
  The Animation hasn't been replaced.
  The member would have to be renamed to something else... not sure what.

• There's no way to restore the animation from 'removed' to the 'ok' state, only to the 'persisted' state.
  That makes sense for an animation that has been replaced before but not for an animation on which one has simply called commitXXX().

• It wouldn't make sense to become removed when using the version of commitXXX that takes a list of property names, so for consistency it should not become removed when no property names are specified.

3. Should it flush style?

This is really tricky.

Flushing style will produce a more accurate result. For example, if there is an animation of font-size on an ancestor, we should flush that before calculating the final computed style so that the updated font-size is reflected.

However, this method mutates specified style. If it also flushes style, then, for example, calling commitXXX() on a series of animations in the same tick will produce poor performance as it successively flushes and invalidates styles.

For now I've gone with not flushing style since that seems preferably to me and is consistent with all the other methods on the Animation interface.

4. Are we sure we want to commit computed styles?

I thought about this for a while and I think, yes, we do.

It's really tempting to try to make this method preserve the specified values in all their context-sensitive richness. For example, we could:

• Define an interpolate(from, to, p) function in CSS
• Making commitXXX() produce interpolate() functions as needed
• Not ship additive animation until we have suitable syntax for expressing it in static CSS (e.g. the !add proposal) -- since if an stack of additive animation includes context-sensitive values like CSS variables and em-based units we need to preserve the stack in order to produce the correct result.

However, not only does that still fail to cover the case of multiple animations using implicit from/to keyframes filling mid-interval (my favourite edge case it seems) it also doesn't really cover additive animations properly even once we have !add.

Even with !add you would still end up leaking memory since you'd end up appending the additive values to the inline style declaration in order to produce the desired additive effect. But the whole point of this method is that it collapses the result. The only way we can do that is to pre-combine the result, even though that necessarily means losing some information.

So I think it's probably fine to say: Use persist() when you need 100% fidelity, otherwise use commitXXX().

5. What about pseudo elements?

There is no inline style we can write to for these. Perhaps that's ok?

Pseudo elements are mostly used in conjuction with CSS animations/transitions and we don't do replacement for CSS animations/transitions so this is not likely to come up often in practice.

Even if we decide that's ok, should commitXXX() throw when the target is a pseudo element? (And if so, should it also throw if there is no effect? No target element?)

6. What should it do for non-rendered targets?

Similar to the previous question, if the target is a display:none how should this work? Even if we can compose a suitable animation style, there is at least the problem of how to convert logical properties to physical properties in the list of properties passed to commitXXX() without a context to resolve them against. In my current implementation I've just made it silently fail for all these cases.

[birtles]

From my experience implementing this alternative approach:

• It took only about 3 days to implement, much of which was writing WPT (compared to several weeks for the FillAnimation proposal)
• Not a single existing test in WPT or Gecko's testsuite failed with this change (i.e. compat risk should be low)
• It improves Gecko's performance on https://greensock.com/js/speed.html so that the frame rate no longer drops over time (something the FillAnimation approach only partially addressed)

[birtles]

There is a PR for this over in #3833 but I've encountered one issue regarding commitStyles when updating my WIP implementation to match that PR.

Initially I argued that commitStyles should not flush styles first: having a method that both flushes and mutates style means that if you call commitStyles on a number of animations within the same frame, you end up flushing styles a lot and there's really nothing the author can do to avoid it.

In that PR I also specified that commitStyles should throw if any of the target elements was not being rendered since we won't computed style to build on (and we won't have a computed writing-mode etc. in order to map logical properties to physical properties when the author specifies a list of properties).

However, that means that simple test cases like the following don't work:

const div = createDiv(t);
const animation = div.animate({ opacity: 0 },  { duration: 1, fill: 'forwards' });
animation.finish();
animation.commitStyles();

At the point where we call commitStyles() we still haven't resolved style for div so we will think it is not rendered and hence throw.

I think it might be better to make commitStyles() flush style after all. Not only will it make the above test case work as expected, it will also produce less confusing results when style has not been updated yet -- i.e. you'll get the styles for this frame, not the previous frame.

It's not great for performance, but at least UAs only need to flush style, not layout.

I'd be keen to know what others think about this. I imagine @emilio might have some ideas.

(The only other alternative I can think of to this API that avoids this issue and accommodates GroupEffects is to record the effective animation values on the remove event by using some sort of map from target elements to computed style. We could possibly even add a method that takes such an event or its map and applies it to inline style directly--but all that sounds very clumsy to me.)

[emilio]

I'm not particularly excited about the need to flush in there... Is the computed style only needed for logical properties? I wonder if we should be preserving them later, or deferring the commitStyles somehow...

It's not great for performance, but at least UAs only need to flush style, not layout.

Note that last time I checked Blink and WebKit don't have that difference between "just update styles" and "update style and layout".

[birtles]

I'm not particularly excited about the need to flush in there... Is the computed style only needed for logical properties?

The computed style is only needed for logical properties, but the flush is needed to ensure we commit the up-to-date state.

For example:

div.style.fontSize = '10px';
const anim = div.animate(
  { width: '100em' },
  { duration: 1000, fill: 'forwards' }
);
anim.ready.then(() => {
  anim.finish();
  div.style.fontSize = '20px';
  anim.commitStyles();
  anim.cancel();
  console.log(getComputedStyle(div).width);
});

Without the flush, this will produce 1000px instead of 2000px. (Furthermore, we have to wait at least a frame after loading or else commitStyles will throw since div is still not being rendered, hence the waiting on ready above. If we flush we don't need to do that.)

A more likely case is something like:

container.style.fontSize = '10px';
const containerAnim = container.animate(
  { fontSize: '20px' },
  {
    duration: 1000,
    fill: 'forwards',
    easing: 'step-end',
  }
);
const childAnimA = child.animate(
  { width: '100em' },
  { duration: 1000, fill: 'forwards' }
);
const childAnimB = child.animate(
  { width: '100em' },
  { duration: 1000, fill: 'forwards' }
);
childAnimA.onremove = () => {
  childAnimA.commitStyles();
  childAnimB.cancel();
  console.log(getComputedStyle(div).width);
};

This too will produce 1000px without a flush but 2000px with the flush (since animation events, like rAF callbacks, run before we update style for the frame).

I wonder if we should be preserving them later, or deferring the commitStyles somehow...

Deferring the commitStyles sounds interesting. I'm not sure how that would work though. (In particular I wonder how it would interact with additive effects since, if you have a bunch of additive effects finishing at once, you need them to be committed in turn.)