-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
test_no_interpolation tests don't have the right expectations for transitionable properties that have value-pairs that interpolate discretely #39871
Comments
cc @josepharhar |
Here's a reduced testcase to get to the crux of the matter here, with a hover-triggered restyle of In this testcase, I would expect that cases (1) and (2) should behave the same as each other (with a 50% flip), and cases (3) and (4) should behave the same as each other (with a smooth transition). I expect those pairs to behave the same because they only differ in whether they use However, WPT test http://wpt.live/css/css-backgrounds/animations/background-size-interpolation.html is essentially expecting that cases (1) and (2) should behave differently, since (as with the other properties in my initial comment here) it's expecting an immediate style-change for non-interpolatable values iff |
If my understanding is correct, then I think probably the fix here would be for the |
Hi, I made the changes to test_no_interpolation recently. Requiring the property to be explicitly listed to opt into this new and potentially breaking behavior, instead of also changing everything that uses transition:all, will help us mitigate damage to websites. Here is a regression bug from me trying to ship the new discrete transition behavior due to changes when It may turn out that we can't ship this new discrete transition behavior at all, but I am still working on figuring that out. |
That sounds like we need to update the spec to better-match what-is-possible-to-ship, then. |
Do you have a timeline on figuring that out? We've got an implementation that's nearly ready to land in https://bugzilla.mozilla.org/show_bug.cgi?id=1805727 . If we're |
I am going to do some preliminary research this week to see if it is for sure unshippable. If we decide to go forward with it, then I wouldn't be 100% certain that it would be compatible until it gets to stable chrome 115, which would happen on July 18th. |
Thanks! |
@dholbert here is an issue discussing this: w3c/csswg-drafts#8857 |
Thanks. For the purposes of this WPT github-issue here (about the tests agreeing/disagreeing with the spec)... While the hypothetical new syntax is being hashed out, could all of the These tests are all contributing to interop-2023 score deficits, and from an "interop in 2023" perspective, that doesn't really make sense if the new behavior is going to behind a new syntax that's not used anywhere on the web yet. When we were considering changing the default interpolation behavior, it made sense to update these tests' expectations; but now that we're leaving the default behavior as-is, it feels like we should revert them, and either add a new test or extend these existing tests (after reverting them) once we've got the new syntax hashed out. |
This was asked for here: #39871 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Oh sorry I didn't know that the affected tests were in interop23. Would this change be enough? #40177 |
This was asked for here: #39871 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
@josepharhar Thanks -- I didn't look too closely, but I see Boris did, and I think that generally addresses the thing I was asking about. I took a quick look at some of the results on the staging server here... |
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, this patch disables CSSTransitionDiscrete and undoes the WPT changes. The affected WPTs are interop23 tracked so the other browsers would like them to be updated ASAP. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, this patch disables CSSTransitionDiscrete and undoes the WPT changes. The affected WPTs are interop23 tracked so the other browsers would like them to be updated ASAP. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Bug: 1453112 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: Robert Flack <[email protected]> Cr-Commit-Position: refs/heads/main@{#1160284}
Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: #39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: Robert Flack <[email protected]> Cr-Commit-Position: refs/heads/main@{#1160284}
I finally finished undoing the test changes, sorry it took so long. |
Thanks! Yup, I think we're all set here. I'll let you know if I run across anything unexpected. |
@josepharhar I found one thing left over: Here's a historical link to wpt.fyi results for the test, before the discrete-interpolation changes, where Firefox passes all subtests (probably Safari as well, though they don't have results recorded in wpt.fyi): Comparing the historical version against the current version, there are two sets of differences, both of which were added as part of various discrete-transitions churn there: I think both of those changes are bogus at this point and the test probably wants to be reverted entirely to its historical ad172b1 state -- does that make sense? (FWIW we're tracking these subtest failures in https://bugzilla.mozilla.org/show_bug.cgi?id=1840438 ) |
Sorry about that, I believe this PR should change the test back to normal: https://github.com/web-platform-tests/wpt/pull/40551/files#diff-cd8c420542d6833285c2aa4af0724735203b305fd9ada3257a28a49f800738ab |
Thanks! The changes to this test there look good to me; I see Firefox passing 36/36 subtests in the wpt.fyi view of the PR test-results: \o/ |
… test changes, a=testonly Automatic update from web-platform-tests Undo CSSTransitionDiscrete interpolation test changes Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: web-platform-tests/wpt#39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: Robert Flack <[email protected]> Cr-Commit-Position: refs/heads/main@{#1160284} -- wpt-commits: 0aa3fc0591ed7ec5b15a1186680b0010508d8700 wpt-pr: 40177
… test changes, a=testonly Automatic update from web-platform-tests Undo CSSTransitionDiscrete interpolation test changes Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: web-platform-tests/wpt#39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <jarharchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Reviewed-by: Robert Flack <flackrchromium.org> Cr-Commit-Position: refs/heads/main{#1160284} -- wpt-commits: 0aa3fc0591ed7ec5b15a1186680b0010508d8700 wpt-pr: 40177 UltraBlame original commit: ff97c0f80a1777f0b37bad0db2b421cbbfeccc07
… test changes, a=testonly Automatic update from web-platform-tests Undo CSSTransitionDiscrete interpolation test changes Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: web-platform-tests/wpt#39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <jarharchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Reviewed-by: Robert Flack <flackrchromium.org> Cr-Commit-Position: refs/heads/main{#1160284} -- wpt-commits: 0aa3fc0591ed7ec5b15a1186680b0010508d8700 wpt-pr: 40177 UltraBlame original commit: ff97c0f80a1777f0b37bad0db2b421cbbfeccc07
… test changes, a=testonly Automatic update from web-platform-tests Undo CSSTransitionDiscrete interpolation test changes Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: web-platform-tests/wpt#39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <jarharchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Reviewed-by: Robert Flack <flackrchromium.org> Cr-Commit-Position: refs/heads/main{#1160284} -- wpt-commits: 0aa3fc0591ed7ec5b15a1186680b0010508d8700 wpt-pr: 40177 UltraBlame original commit: ff97c0f80a1777f0b37bad0db2b421cbbfeccc07
… test changes, a=testonly Automatic update from web-platform-tests Undo CSSTransitionDiscrete interpolation test changes Now that we are planning to add a new opt-in syntax for CSSTransitionDiscrete, the test changes which I made for CSSTransitionDiscrete will not eventually stick. Since these tests are interop23 tracked, the other browsers would like them to be updated ASAP. I thought about also disabling the CSSTransitionDiscrete flag, which would make this patch a lot cleaner, but that would break demos and stuff people are working on so I decided against it. This was asked for here: web-platform-tests/wpt#39871 CSSTransitionDiscrete opt in syntax: w3c/csswg-drafts#8857 Change-Id: If28b7e9fc4fc0d6dc851e35007fd7614a9b497a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4558664 Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: Robert Flack <[email protected]> Cr-Commit-Position: refs/heads/main@{#1160284} -- wpt-commits: 0aa3fc0591ed7ec5b15a1186680b0010508d8700 wpt-pr: 40177
I think there's a test bug (and a Chromium bug) in a bunch of interpolation WPT tests that use
test_no_interpolation()
with discretely-interpolatable values of a property that is otherwise generally interpolatable.For example, "caret-color" (which takes colors and hence can generally interpolate) and "offset-rotate" (which takes angles and can generally interpolate), whose interpolation is tested in these tests:
http://wpt.live/css/css-ui/animation/caret-color-interpolation.html
http://wpt.live/css/motion/animation/offset-rotate-interpolation.html
General issue / tl:dr: right now, these^ tests expect
transition:all
to magically include or exclude these properties depending on whether or not the particular begin/end values happen to be discretely vs. smoothly interpolatable. And I don't think that matches the csswg resolution or spec edit -- the spec talks about the property as a whole being included intransition:all
depending on whether the property as a whole has a discrete animation type.Quoting some relevant bits of the test expectations (from running them on wpt.live):
For a given pair of values, the tests are expecting a discrete 50% flip for
transition: <property name>
BUT they're expecting an immediate value-change (no transition) fortransition: all
. I don't think that matches the spec.Spec reference:
https://drafts.csswg.org/css-transitions-2/#transition-property-property
In this case, the property's animation type is not discrete[1], so it should be included in
transition:all
. (It does have some keyword values likeauto
that aren't transitionable, but the value as a whole still has a non-discrete
Animation Type.@fantasai, @dbaron, am I understanding this spec text (from w3c/csswg-drafts#8520 ) correctly? Do we need to fix the tests, or is the spec missing some nuance?
(Note that Chromium passes these tests which I think means Chromium has a bug. Firefox starts failing the above-quoted
transition: all
subtests with our patch to add the 50% flip, in https://bugzilla.mozilla.org/show_bug.cgi?id=1805727 -- but as I understand it, our test-failures are actually what the spec requires.)[1] e.g. https://drafts.csswg.org/css-ui-3/#caret-color has "Animation type: by computed value" (not "discrete"). Note that "by computed value" is defined such that certain values "combine as discrete" but the property's animation type is still "by computed value".
The text was updated successfully, but these errors were encountered: