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.

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

[css-animations] Handling values where endpoints are valid, but midpoints are IACVT #10369

Open
tabatkins opened this issue May 24, 2024 · 3 comments

Comments

@tabatkins
Copy link
Member

In #10220 we have a situation where the two endpoints of a width interpolation, say, might be large-but-valid calc-size() functions, but the interpolation midpoints are invalid due to the calculation getting too large. I'm handling that "too large" case by saying the property becomes invalid-at-computed-value-time, since it's essentially the same issue as variable substitution, which can also become too large via billion-laugh attacks.

This produces a new case that was previously not possible in animations, where two values are valid to interpolate, but their mid-interpolation value isn't valid. In Chrome's implementation, @dbaron isn't sure how to even handle that, and is currently instead doing something simpler.

How should we handle this? I'm inclined to say that the animation should fall back to discrete in this case, since the two endpoints are, by definition, valid.

@tabatkins tabatkins added the css-animations-1 Current Work label May 24, 2024
@birtles birtles added web-animations-1 css-values-4 Current Work and removed css-animations-1 Current Work labels May 28, 2024
@birtles
Copy link
Contributor

birtles commented May 28, 2024

Falling back to discrete makes sense to me. The spec text for this should likely go either in CSS Values under § 3. Combining Values where we define interpolation including handling out-of-range values, or in Web Animations under § 5.2 Animating Properties where we define discrete animation and how to choose a particular animation type.

Edit: Might it even be as simple as extending this sentence: "If the number of components or the types of corresponding components do not match, or if any component value uses discrete animation and the two corresponding values do not match, then the property values combine as discrete." ?

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-animations] Handling values where endpoints are valid, but midpoints are IACVT, and agreed to the following:

  • RESOLVED: if the interpolation endpoints are validly interpolable but the intermediate points are IACVT then we treat the entire interpolation as discrete
The full IRC log of that discussion <jarhar> TabAtkins: while specifying calc size, i realized in order to get it to work right i had to do some thing sthat make calc size vulnerable to ?
<jarhar> TabAtkins: i put in saying protection fo rhtat, if your calc size expands to a megabyte thats your problem and invalidate the thing. that does mean that you could have a valid calc size before and after interpolation, but makes it invalid during computed value time. only place where property can become invalid mid interpolation
<jarhar> TabAtkins: i dont know if this will happen again, but we need some definition for what it means for interpolation where intermediate values are ?
<jarhar> TabAtkins: after discussing with dbaron, the most obvious answer is that if that happens then the transition falls back to discrete
<jarhar> TabAtkins: assumes that ? applies to all values, not one particular value or point, thats the case right now, substituion will happen for the entire interpolation. if it comes to the case for an inteprolation later thats invalid, we will think about it then and push that to future us
<ydaniv> q+
<jarhar> TabAtkins: for now, the only case that matters is its invalid for the entire thing
<jarhar> TabAtkins: i suggest that we resolve that in this case the property interpolation reverts to discrete
<astearns> ack dbaron
<jarhar> dbaron: when you say it falls back to discrete, is there any disttinction of that wording vs you cant interpolate?
<jarhar> TabAtkins: yes because we do have a distinction between discrete and not transition
<jarhar> dbaron: discrete transition means what happens when you cant interpolation
<jarhar> TabAtkins: we have smoe values that cant be interpolated, they dont interact with interpolatin system. distinct from not being able to ? a pair
<jarhar> dbaron: i would have used the term "ability to animate" and "ability to interpolate" in the second case
<jarhar> TabAtkins: not sure if im using right spec terms
<jarhar> dbaron: me neither
<jarhar> dbaron: i would have described this as ? interpolate between the values. we are using different words for the same thing
<astearns> ack ydaniv
<jarhar> ydaniv: would it be possible to just drop the ? where its not possible to get a value and just keep the rest possible value until you pass that point?
<jarhar> TabAtkins: the situation that ? out of here is that the entire interpolation
<jarhar> TabAtkins: all points after the start ? be the same and trigger the errror case, ?? stick with until we ??
<jarhar> TabAtkins: if the interpolatin endpoints are validly interpolable but the intermediate points are ? then we treat the entire interpolation as discrete
<jarhar> astearns: you said all intermediate values
<jarhar> TabAtkins: we dont have a case where it can be ? righ tnow
<jarhar> TabAtkins: only some intermediate values invalid
<astearns> s/?/IACVT/
<jarhar> astearns: should be invalid at computed value time
<jarhar> astearns: any objections to this proposed resolution?
<astearns> proposed resolution: if the interpolatin endpoints are validly interpolable but the intermediate points are IACVT then we treat the entire interpolation as discrete
<jarhar> fserb: is there a reason why we dont like the wording on the issue itself? i think someone which is a - what you said is a little bit more than whats said there. you're talking about values, and its talking about whats the cause of the values to not be combinable
<jarhar> TabAtkins: i dont see where im saying different
<jarhar> fserb: when youre saying when the result of the combination is invalid, ? which would mean that the combination is invalid
<jarhar> TabAtkins: thats not what im saying
<jarhar> TabAtkins: if youre looking at ?'s comment, hes talking about a part of the spec we would add to, thats a different thing, ? discrete fallback
<jarhar> astearns: we are resolved
<TabAtkins> s/?'s comment/birtle's comment/
<astearns> RESOLVED: if the interpolation endpoints are validly interpolable but the intermediate points are IACVT then we treat the entire interpolation as discrete
<dbaron> s/birtle's comment/birtles's comment/

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 16, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 17, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}
sadym-chromium pushed a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 19, 2024
…c-size() interpolation would exceed expansion limit., a=testonly

Automatic update from web-platform-tests
Fall back to discrete animation when calc-size() interpolation would exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}

--

wpt-commits: bfdf78dc0dcf0336b5dd3faf50895c2d7a1874ff
wpt-pr: 47168
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jul 19, 2024
…c-size() interpolation would exceed expansion limit., a=testonly

Automatic update from web-platform-tests
Fall back to discrete animation when calc-size() interpolation would exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}

--

wpt-commits: bfdf78dc0dcf0336b5dd3faf50895c2d7a1874ff
wpt-pr: 47168
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jul 20, 2024
…c-size() interpolation would exceed expansion limit., a=testonly

Automatic update from web-platform-tests
Fall back to discrete animation when calc-size() interpolation would exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}

--

wpt-commits: bfdf78dc0dcf0336b5dd3faf50895c2d7a1874ff
wpt-pr: 47168
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Jul 22, 2024
…exceed expansion limit.

Interpolating between calc-size() expressions that have a nested
calc-size() expression as the basis requires expansion in order to make
the bases compatible.  We have an expansion limit to prevent overly
large results.

This change makes expressions whose interpolation would exceed the
expansion limit be treated as not interpolable (that is, animate
discretely), as resolved in
w3c/csswg-drafts#10369 (comment)

Fixed: 346975480
Bug: 313072
Change-Id: I22da5a67eddb429140e813e0b41a7af641b8fa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5714768
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329123}
@dbaron
Copy link
Member

dbaron commented Aug 16, 2024

@tabatkins edited the calc-size-specific parts of this in 7d66305 (and there's no longer any actual IACVT behavior there). I'm not sure if more general animations edits are needed here or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Thursday morning
Development

No branches or pull requests

5 participants