[web-animations-1] commitStyles followed by cancel should not trigger a transition

[birtles]

In Mozilla bug 1917071 we came across a case where Gecko is firing transitions as a result of calling commitStyles.

Specifically, for the following content:

#box {
  opacity: 0;
  transition: opacity 1s;
}

document.querySelector('button').onclick = () => {
  let animation = document
    .querySelector('#box')
    .animate({ opacity: 1 }, { fill: 'forwards', duration: 1000 });
  animation.onfinish = () => {
    animation.commitStyles();
    animation.cancel();
  };
};

After analysis, it looks like the problem is that although commitStyles is defined as triggering a style flush:

Unlike most other methods defined on this interface, calling this method does trigger a style change event (see § 6.13 Model liveness).

It doesn't define when this takes place. Logically it needs to happen towards the start of the procedure so that the latest styles are applied. The code for Gecko and Blink shows they flush style before updating inline style.

Edit: It looks like the spec does define that pending style changes are applied at the start of the procedure ("If, after applying any pending style changes...").

However, there is no requirement to flush styles at the end of the procedure. Furthermore, my reading of the procedure to update the style attribute doesn't require computing style immediately. As a result, the style attribute may be updated but not yet reflected in the corresponding element's computed style.

If the animation is canceled before style is next computed, the changes to inline style will not be reflected in the before-change style since it only requires updating computed values from declarative animations. As a result, canceling the animation can cause a change to be observed and transitions to be fired.

I'm not exactly sure why the bug doesn't reproduce in Blink. Perhaps it is reflecting the changes to the style attribute as part of producing the before-change style?

I think we need to specify that commitStyles should somehow cause the computed style to be synchronously updated as a result of updating the style attribute—or somehow indicate that commitStyles followed by cancelling an animation should not trigger a transition. We may need to refine this wording when we tackle #5394, however.

/cc @flackr @graouts @emilio @canalun @vmpstr

[birtles]

Also, apparently Safari shows the same behavior as Gecko for this case.

[graouts]

I can confirm that WebKit performs a flush immediately after it was established that we can proceed with this method and decided not to throw a NoModificationAllowedError exception. There is no further flush performed within this procedure.

[flackr]

In the starting of transitions, it says:

Likewise, define the after-change style as the computed values of all properties on the element based on the information known at the start of that style change event, but using the computed values of the animation-* properties from the before-change style, excluding any styles from CSS Transitions in the computation, and inheriting from the after-change style of the parent. Note that this means the after-change style does not differ from the before-change style due to newly created or canceled CSS Animations.

To me, using the the animation-* properties from the before-change style suggests that the intent here is that the after-change style should include the canceled animation in this case, resulting in no change on the style update for which the animation was canceled. Of course this should be spelled out explicitly if this is the way we want to resolve this, but it seems consistent with the way we would prevent transitions starting on style changes applied at the same time as a css animation is removed.

[flackr]

I'm not exactly sure why the bug doesn't reproduce in Blink. Perhaps it is reflecting the changes to the style attribute as part of producing the before-change style?

This seems to be why we don't start a transition in blink, we still consider the canceled animation in CSSAnimations::CanCalculateTransitionUpdateForProperty. This is I think consistent with my suggestion above, #11084 (comment), as if the animation were still active it would produce the same before and after change style.

[birtles]

To me, using the the animation-* properties from the before-change style suggests that the intent here is that the after-change style should include the canceled animation in this case, resulting in no change on the style update for which the animation was canceled.

I see. I can understand that for animations created/cancelled by animation-* properties because it's the style change that actually cancels them. But these animations have already been canceled prior to any style change event. Temporarily un-canceling them seems like something different?

[birtles]

I'm going to try to set aside a bit of time in the next day or two to run a few tests to see if I can convince myself of the Chrome behavior or otherwise work out what's reasonable to spec.

[flackr]

But these animations have already been canceled prior to any style change event. Temporarily un-canceling them seems like something different?

I think of it as we haven't had a style change event where their previous styles could actually be undone yet, so even though they're canceled we should still consider the styles they had been applying with the next style change event.

[flackr]

Seems like this relates to #6688 where I also brought up that this shouldn't trigger a transition - #6688 (comment)