Closed Bug 1973203 Opened 2 months ago Closed 2 months ago

Enable commitStyles behavior by default on release channels

Categories

(Core :: DOM: Animation, task)

task

Tracking

()

RESOLVED FIXED
142 Branch
Tracking Status
firefox142 --- fixed

People

(Reporter: canalun, Assigned: canalun)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

The endpoint-inclusive commitStyles behavior is confirmed as web-compat through the test on Nightly. (https://github.com/w3c/csswg-drafts/issues/5394#issuecomment-2954654330)
And the specification has been finalized. (https://github.com/w3c/csswg-drafts/pull/11881).

So this task enables the behavior by default on the release channel by setting the pref.
The corresponding Web Platform Test testing/web-platform/tests/editing/run/commitStyles.tentative.html is renamed to commitStyles.html also.

Depends on: 1961558
Keywords: dev-doc-needed

This task enables the behavior by default on the release channel by setting the pref. ALso it replaces the corresponding wpt case to new one.

Assignee: nobody → i.am.kanaru.sato
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53342 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 142 Branch
Upstream PR merged by moz-wptsync-bot

:canalun is this something you want to mention in the Fx142 release notes? Please nominate if so.

Flags: needinfo?(i.am.kanaru.sato)

:diannaS

I think we don't have to list it on the release notes.
Instead, I added dev-doc-needed for the following points :)

  • Browser Compat Data
  • MDN of commitStyles: Specifically, it's better to tweak "Examples" because the current content implicitly assumes the previous behavior of this API. And if we do that, we also need to mention that there is a difference between Firefox and other browsers now.
Flags: needinfo?(i.am.kanaru.sato)
QA Whiteboard: [qa-triage-done-c143/b142]

FF142 MDN docs for this can be tracked in https://github.com/mdn/content/issues/40482

As I understand it, if you call Animation.commitStyles() while in the active interval, the value written to the element style will be whatever the current computed values are at that time.

If you call the method after the active state, previously the method was endpoint-exclusive. Since the fill applies in the next phase, if you haven't defined a fill: 'forwards' the fill value isn't available? [is that right? what actually happens]?

But now with the spec change, the active state is endpoint-inclusive. So the state gets saved at the end of the active phase, and irrespective of the fill state setting or whether the animation has been removed, that state can be written.

To summarize, the difference is that you no longer need to specify fill:forwards for commitStyles() to save the styles after the end of the animation. is that right?
Further, since you don't have to set the fill, you wouldn't be wasting resources at the end of the animation, but presumably you do have to be careful to call this as soon after the animation ends as possible - right?

So the change here would be update https://developer.mozilla.org/en-US/docs/Web/API/Animation/commitStyles#examples to make it clear you don't need to set the fill? And perhaps to adjust the comment at the end of this section https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API#persisting_animation_styles ?

EDIT As an aside, why is fill: 'back' not relevant? I mean doesn't an animation playing backwards need to set its final value too?

Flags: needinfo?(i.am.kanaru.sato)

Following up for :canalun here because I authored the spec change.

(In reply to Hamish Willee from comment #8)

As I understand it, if you call Animation.commitStyles() while in the active interval, the value written to the element style will be whatever the current computed values are at that time.

Correct.

If you call the method after the active state, previously the method was endpoint-exclusive. Since the fill applies in the next phase, if you haven't defined a fill: 'forwards' the fill value isn't available? [is that right? what actually happens]?

Correct. (The "finishing" behavior means that when the animation plays to either end of the active interval, it stops there. But the endpoint exclusive behavior would mean that, in the absence of a fill mode, you'd end up with an unresolved time value and hence no computed animation value. This comes from https://drafts.csswg.org/web-animations-1/#calculating-the-active-time and the phase definitions it references.)

But now with the spec change, the active state is endpoint-inclusive. So the state gets saved at the end of the active phase, and irrespective of the fill state setting or whether the animation has been removed, that state can be written.

More or less. Basically we've added a flag, used exclusively by commitStyles, which tweaks the bounds of the phases when calculating the computed values applied by commitStyles so that, regardless of the fill mode, at the active interval boundaries you still get computed animation values.

To summarize, the difference is that you no longer need to specify fill:forwards for commitStyles() to save the styles after the end of the animation. is that right?

Yes, that's correct.

You might find the spec issue (https://github.com/w3c/csswg-drafts/issues/5394) or the spec PR (https://github.com/w3c/csswg-drafts/pull/11881/files) helpful.

Further, since you don't have to set the fill, you wouldn't be wasting resources at the end of the animation, but presumably you do have to be careful to call this as soon after the animation ends as possible - right?

Yes, fill modes are discouraged by the spec (see note at https://drafts.csswg.org/web-animations-1/#fill-mode).

However, you don't need to be too careful about calling commitStyles quickly as the "finishing" behavior of animations should mean the play head for the animation stops at the end of the active interval. As a result, if you call it a frame or two later, it should still produce the correct result.

However, if there are multiple animations that layer on top of one another, ordering could make a difference to the result, so generally you'll want to call commitStyles immediately after the animation ends (e.g. by waiting on the finished promsie).

So the change here would be update https://developer.mozilla.org/en-US/docs/Web/API/Animation/commitStyles#examples to make it clear you don't need to set the fill?

Yes. The spec PR (https://github.com/w3c/csswg-drafts/pull/11881/files) might be helpful since it updates similar examples.

And perhaps to adjust the comment at the end of this section https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API#persisting_animation_styles ?

I think that section looks ok? What would the change be?

EDIT As an aside, why is fill: 'back' not relevant? I mean doesn't an animation playing backwards need to set its final value too?

In which part is fill: backwards not relevant? I think both the spec and code change cover when the animation is playing backwards too?

Flags: needinfo?(i.am.kanaru.sato)

Thanks Brian - that's very helpful.

Last questions:

  • The recommendation is not to set a fill, but if you don't then this won't work on older browsers. Is there an appropriate fallback approach?
  • We'll need some kind of compatibility data update to indicate this. Do you think a name around fill styles not needed would be reasonable as a key/description?

FYI

EDIT As an aside, why is fill: 'back' not relevant? I mean doesn't an animation playing backwards need to set its final value too?
which part is fill: backwards not relevant? I think both the spec and code change cover when the animation is playing backwards too?

I think I just misinterpreted a few comments such as https://github.com/w3c/csswg-drafts/issues/5394#issuecomment-672394173 to mean "only forwards or both because it includes forwards" rather than "this example only goes forwards so both or forwards make sense".

So my very first comment should have been:

If you call the method after the active state, previously the method was endpoint-exclusive. Since the fill applies in the next phase, if you haven't defined a fill: 'forwards' fill that matches the direction of animation the fill value isn't available?

And perhaps to adjust the comment at the end of this section https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API#persisting_animation_styles ?
I think that section looks ok? What would the change be?

Add " or backwards, depending on the direction of animation. " to the end of this first paragraph?

When animating elements, a common use case is to persist the final state of the animation, after the animation has finished. One method sometimes used for this is to set the animation's fill mode to forwards.

Flags: needinfo?(brian)

No problem!

(In reply to Hamish Willee from comment #10)

Thanks Brian - that's very helpful.

Last questions:

  • The recommendation is not to set a fill, but if you don't then this won't work on older browsers. Is there an appropriate fallback approach?

I think existing code will continue to work. So I think until this spec change has been made in all engines and sufficiently deployed, it's best to keep the old approach (use a fill mode and then make sure to explicitly cancel the animation after it finishes).

  • We'll need some kind of compatibility data update to indicate this. Do you think a name around fill styles not needed would be reasonable as a key/description?

I think that might be a bit broad—there might still be some cases where fills are unavoidable, especially for backwards fills. Perhaps "endpoint-inclusive commitStyles"? "commitStyles automatically fills values"?

Flags: needinfo?(brian)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: