Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[web-animations-1] Initialize start time of scroll animations to zero (#2075) #4842

Merged
merged 6 commits into from
Apr 13, 2020
Merged

[web-animations-1] Initialize start time of scroll animations to zero (#2075) #4842

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1fd6277
Initial change.
ogerchikov Mar 6, 2020
fc80ef2
Initialize start time of scroll animations to zero.
ogerchikov Mar 27, 2020
3df0083
Use monotonically increasing generalization of timelines instead of f…
ogerchikov Apr 2, 2020
441a771
Update play states.
ogerchikov Apr 8, 2020
5d11616
Addressed code review comments.
ogerchikov Apr 9, 2020
4806500
Addressed code review comments.
ogerchikov Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 106 additions & 47 deletions web-animations-1/Overview.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,31 +828,6 @@ follows:
<var>animation</var> with the <var>did seek</var> flag set to false, and
the <var>synchronously notify</var> flag set to false.

<h4 id="responding-to-a-newly-inactive-timeline">Responding to a newly inactive
timeline</h4>

Issue(2080): With the set of timelines defined in this level of this
specification, this situation is not expected to occur. As
a result, this section will likely be moved to a subsequent level
of this specification.

When the <a>timeline</a> associated with an <a>animation</a>,
<var>animation</var>, becomes newly <a lt="inactive timeline">inactive</a>,
if <var>animation</var>'s <a>previous current time</a> is <a
lt="unresolved">resolved</a>, the procedure to <a>silently set the current
time</a> of <var>animation</var> to <a>previous current time</a> is run.

<div class="note">

This step makes the behavior when an <a>animation</a>'s <a>timeline</a> becomes
<a lt="inactive timeline">inactive</a> consistent with when it is
disassociated with a <a>timeline</a>.
Furthermore, it ensures that the only occasion on which an <a>animation</a>
becomes <a lt="idle play state">idle</a>, is when the procedure to
<a>cancel an animation</a> is performed.

</div>

<h4 id="setting-the-associated-effect" oldids="setting-the-target-effect">Setting the associated effect of an animation</h4>

The procedure to <dfn>set the associated effect of an animation</dfn>,
Expand Down Expand Up @@ -1146,6 +1121,9 @@ as CSS Animations [[CSS-ANIMATIONS-1]].
<var>animation</var> has a <a>pending pause task</a>, and false otherwise.
1. Let <var>has pending ready promise</var> be a boolean flag that is
initially false.
1. Let <var>performed seek</var> be a boolean flag that is initially false.
1. Let <var>has finite timeline</var> be true if |animation| has an associated
<a>timeline</a> that is not [=monotonically increasing=].
1. Perform the steps corresponding to the <em>first</em> matching
condition from the following, if any:

Expand All @@ -1159,23 +1137,65 @@ as CSS Animations [[CSS-ANIMATIONS-1]].
* <a>current time</a> &lt; zero, or
* <a>current time</a> &ge; <a>associated effect end</a>,

:: Set <var>animation</var>'s <a>hold time</a> to zero.
:: 1. Set <var>performed seek</var> to true.
1. Update either <var>animation</var>’s <a>start time</a> or
<a>hold time</a> as follows:

<div class="switch">
birtles marked this conversation as resolved.
Show resolved Hide resolved

: If |has finite timeline| is true,
:: Set <var>animation</var>'s <a>start time</a> to zero.
: Otherwise,
:: Set <var>animation</var>'s <a>hold time</a> to zero.

</div>

: If |animation|'s [=effective playback rate=] &lt; 0,
the <var>auto-rewind</var> flag is true and <em>either</em>
<var>animation</var>'s:

* <a>current time</a> is <a>unresolved</a>, or
* <a>current time</a> &le; zero, or
* <a>current time</a> &gt; <a>associated effect end</a>,
::
<div class="switch">

: If <a>associated effect end</a> is positive infinity,
:: <a>throw</a> an "{{InvalidStateError}}" {{DOMException}} and
abort these steps.
: Otherwise,
:: 1. Set <var>performed seek</var> to true.
1. Update either <var>animation</var>’s <a>start time</a> or
<a>hold time</a> as follows:

<div class="switch">

: If |has finite timeline| is true,
:: Set <var>animation</var>'s <a>start time</a> to <a>associated
effect end</a>.
: Otherwise,
:: Set <var>animation</var>'s <a>hold time</a> to <a>associated
effect end</a>.

</div>

</div>

:: If <a>associated effect end</a> is positive infinity,
<a>throw</a> an "{{InvalidStateError}}" {{DOMException}} and
abort these steps.
Otherwise, set <var>animation</var>'s <a>hold time</a> to <a>associated
effect end</a>.
: If |animation|'s [=effective playback rate=] = 0 and |animation|'s
[=current time=] is [=unresolved=],
:: Set <var>animation</var>'s <a>hold time</a> to zero.

:: 1. Set <var>performed seek</var> to true.
1. Update either <var>animation</var>’s <a>start time</a> or
<a>hold time</a> as follows:

<div class="switch">

: If |has finite timeline| is true,
:: Set <var>animation</var>'s <a>start time</a> to zero.
: Otherwise,
:: Set <var>animation</var>'s <a>hold time</a> to zero.

</div>

</div>

Expand All @@ -1185,9 +1205,10 @@ as CSS Animations [[CSS-ANIMATIONS-1]].
1. Cancel that task.
1. Set <var>has pending ready promise</var> to true.

1. If the following three conditions are <em>all</em> satisfied:
1. If the following four conditions are <em>all</em> satisfied:

* |animation|'s [=hold time=] is [=unresolved=], and
* |performed seek| is false, and
* |aborted pause| is false, and
* |animation| does <em>not</em> have a [=pending playback rate=],

Expand Down Expand Up @@ -1320,21 +1341,50 @@ follows:
1. If <var>animation</var> has a <a>pending pause task</a>, abort these steps.
1. If the <a>play state</a> of <var>animation</var> is <a
lt="paused play state">paused</a>, abort these steps.
1. Let <var>has finite timeline</var> be true if |animation| has an associated
<a>timeline</a> that is not [=monotonically increasing=].
1. If the <var>animation</var>'s <a>current time</a> is <a>unresolved</a>,
perform the steps according to the first matching condition from below:

<div class="switch">

: If <var>animation</var>'s [=playback rate=] is &ge; 0,
:: Let <var>animation</var>'s <a>hold time</a> be zero.
: If <var>animation</var>'s [=playback rate=] is &ge; 0,
:: Update either <var>animation</var>’s <a>start time</a> or
<a>hold time</a> as follows:

<div class="switch">

: If |has finite timeline| is true,
:: Set <var>animation</var>'s <a>start time</a> to zero.
: Otherwise,
:: Set <var>animation</var>'s <a>hold time</a> to zero.

</div>

: Otherwise,
::
<div class="switch">

: If <a>associated effect end</a> for <var>animation</var> is positive
infinity,
:: <a>throw</a> an "{{InvalidStateError}}" {{DOMException}}
and abort these steps.
: Otherwise,
:: Update either <var>animation</var>’s <a>start time</a> or
<a>hold time</a> as follows:

<div class="switch">

: If |has finite timeline| is true,
:: let <var>animation</var>'s <a>start time</a> be
<a>associated effect end</a>.
: Otherwise,
:: let <var>animation</var>'s <a>hold time</a> be
<a>associated effect end</a>.

: Otherwise,
:: If <a>associated effect end</a> for <var>animation</var> is positive
infinity,
<a>throw</a> an "{{InvalidStateError}}" {{DOMException}}
and abort these steps.
Otherwise, let <var>animation</var>'s <a>hold time</a> be
<a>associated effect end</a>.
</div>

</div>

</div>

Expand All @@ -1345,10 +1395,14 @@ follows:
1. If <var>has pending ready promise</var> is false,
set <var>animation</var>'s <a>current ready promise</a> to
<a>a new promise</a> in the <a>relevant Realm</a> of <var>animation</var>.
1. Schedule a task to be executed at the first possible moment after
the user agent has performed any processing necessary to suspend
the playback of
<var>animation</var>'s <a>associated effect</a>, if any.
1. Schedule a task to be executed at the first possible moment where
<em>both</em> of the following conditions are true:
* the user agent has performed any processing necessary to suspend
the playback of <var>animation</var>'s <a>associated effect</a>, if any.

* the animation is associated with a <a>timeline</a> that is not
<a lt="inactive timeline">inactive</a>.

The task shall perform the following steps:

1. Let <var>ready time</var> be the time value of the timeline associated
Expand Down Expand Up @@ -1930,11 +1984,14 @@ non-normative description is also provided:

: <a lt="idle play state">idle</a>
:: The <a>current time</a> of the animation is <a>unresolved</a> and
the <a>start time</a> of the animation is <a>unresolved</a> and
there are no pending tasks.
In this state the animation has no effect.
: <a lt="running play state">running</a>
:: The animation has a resolved <a>current time</a> that changes on each
<a>animation frame</a> (provided the [=playback rate=] is not zero).
<a>animation frame</a> (provided the [=playback rate=] is not zero
and the <a>timeline</a> is [=inactive timeline|active=] and
[=monotonically increasing=]).
: <a lt="paused play state">paused</a>
:: The animation has been suspended and the <a>current time</a>
is no longer changing.
Expand All @@ -1953,6 +2010,8 @@ condition from the following:
: <em>All</em> of the following conditions are true:
* The <a>current time</a> of <var>animation</var> is <a>unresolved</a>,
<em>and</em>
* the <a>start time</a> of <var>animation</var> is <a>unresolved</a>,
<em>and</em>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little bit nervous about changing this part since I'm afraid it might have unintended consequences that I haven't been able to anticipate. However, if we can make this change and none of the existing WPT break, then I guess it's ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, having current time unresolved automatically means start time being unresolved. That's how I convinced myself it should not be any backward compatibility problems.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, having current time unresolved automatically means start time being unresolved. That's how I convinced myself it should not be any backward compatibility problems.

Is that true? If you have no timeline or an inactive timeline, I think you can still have a resolved start time with an unresolved current time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After you mentioned null timeline, the case I can think of is when running animation.timeline is set to null. However in Chrome animation.timeline is read only, so hopefully user scripts don't rely on this behavior.

In regards to inactive timelines, I don't think DocumentTimeline is expected to become newly inactive. Even if it does become inactive, the spec says we should use previous resolved time. "Responding to newly inactive timelines" is not implemented in Chrome, so, again, hopefully user scripts don't rely on this behavior.

I think the major impact of this change is that we are changing the mental model of animation play state. Before, 'idle'/'running' implied activeness of a timeline the animation is attached to, while now 'idle' play state can only be achieved by user actions - cancelling a running animation or not calling play/pause.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After you mentioned null timeline, the case I can think of is when running animation.timeline is set to null. However in Chrome animation.timeline is read only, so hopefully user scripts don't rely on this behavior.

I think Chrome is shipping the Animation() constructor, however, which allows creating animations without a timeline so I think it's possible it's already being relied on. It does seem unlikely, however.

* <var>animation</var> does <em>not</em> have <em>either</em>
a <a>pending play task</a> <em>or</em> a <a>pending pause task</a>,
:: &rarr; <dfn lt="idle play state">idle</dfn>
Expand Down