Fix transition toggle & cancellation & delay by yezhizhen · Pull Request #35978 · servo/servo

yezhizhen · 2025-03-14T17:32:18Z

More details in Stylo PR: servo/stylo#145

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes
fixes transition switch to initial value is failed #35833
fixes No delay when transition duration is 0 #35982

There are new passing test: css/css-logical/animation-004.html: Transitions from physical to logical update when the direction is changed

Created new test files as well:

css-transitions/transition-remove-and-change-immediate.html
css-transitions/transition-zero-duration-with-delay.html
css-transitions/transitioncancel-003.html

yezhizhen · 2025-03-18T02:54:58Z

Just rebased on latest main and running new test.
https://github.com/yezhizhen/servo/actions/runs/13914368346

yezhizhen · 2025-03-18T10:21:40Z

https://github.com/yezhizhen/servo/actions/runs/13919914249

Latest successful run. Already applied optimization revamp commit.

servo-wpt-sync · 2025-03-19T04:37:26Z

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#51439) with upstreamable changes.

servo-wpt-sync · 2025-03-19T04:42:29Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

servo-wpt-sync · 2025-03-19T04:42:54Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

servo-wpt-sync · 2025-03-19T04:48:12Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

servo-wpt-sync · 2025-03-19T05:48:34Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-19T05:56:33Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-19T06:00:59Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

servo-wpt-sync · 2025-03-19T06:06:32Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-19T06:09:54Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-19T06:39:46Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-20T06:06:04Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-20T06:06:57Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

servo-wpt-sync · 2025-03-20T06:40:29Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-03-20T07:02:04Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#51439) title and body.

yezhizhen · 2025-03-20T07:04:20Z

https://github.com/yezhizhen/servo/actions/runs/13963105518

I just added tests for all behaviours that was problematic before. CI is passing too.
I don't have further changes in mind right now.

Please review this when you got time :) @Loirooriol

servo-wpt-sync · 2025-04-04T08:32:48Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

Loirooriol

Sorry for the delay reviewing this

tests/wpt/tests/css/css-transitions/transition-remove-and-change-immediate.html

Loirooriol · 2025-04-04T17:07:42Z

tests/wpt/tests/css/css-transitions/transition-zero-duration-with-delay.html

+  assert_approx_equals(
+    elapsed, 300, 30,  // Allow 30ms tolerance
+    'Transition should complete after approximately 300ms'
+  );


I'm concerned that this could be a source of instability. If e.g. the same thread is used to run various tests simultaneously and there are CPU intensive things happening, then I guess the transitionend event could be delivered later.

What's the point of the check, ensuring that the transition doesn't finish immediately? Maybe assert_greater_than would be better?

I checked this to ensure the 0.3s delay is almost accurately executed.
I had this concern before, but later it never seems to happen. But you have the point.

tests/wpt/tests/css/css-transitions/transition-zero-duration-with-delay.html

tests/wpt/tests/css/css-transitions/transitioncancel-003.html

Loirooriol · 2025-04-04T17:22:45Z

tests/wpt/tests/css/css-transitions/transitioncancel-003.html

+  getComputedStyle(div).width;
+  // Start first transition to 100px
+  div.style.width = '100px';
+  // Wait until transition is 25% done (500ms)


Why keep waiting until 25%, and not just until transitionrun?

I just felt natural to write like this. But you are right. I've changed it.

tests/wpt/tests/css/css-transitions/transitioncancel-003.html

servo-wpt-sync · 2025-04-05T03:58:31Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-04-05T04:01:11Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-04-05T05:45:47Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

yezhizhen · 2025-04-05T09:05:39Z

@Loirooriol @xiaochengh
This seems funny. I changed to assert_greater_than. But now both Firefox and Chrome have very unexpected behaviour beyond my imagination. The transition takes less time less than specified combined duration for chrome/firefox sometimes.

Should I assert_greater_than, but with a tiny tolerance now?

Chrome report: very close but still smaller than expected
Firefox report: much smaller than expected

Loirooriol · 2025-04-07T12:07:54Z

Is the timing check an important part of the test? Would it be fine to drop it?

It seems to fit a bit within the umbrella of https://web-platform-tests.org/writing-tests/testharness-api.html#timers-in-tests

In general the use of timers (i.e. setTimeout) in tests is discouraged because this is an observed source of instability on test running in CI.

yezhizhen · 2025-04-07T12:19:57Z

Ok I will drop it.

servo-wpt-sync · 2025-04-07T12:46:59Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

Loirooriol · 2025-04-07T13:51:09Z

It seems DCO check wasn't triggered. I will try a rebase.

servo-wpt-sync · 2025-04-07T13:54:08Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync · 2025-04-07T14:21:11Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51439).

servo-wpt-sync · 2025-04-07T15:25:21Z

⛔ Failed to properly merge the upstream pull request (web-platform-tests/wpt#51439). Please address any CI issues and try to merge manually.

@Asun0204

More details in Stylo PR: servo/stylo#145 ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes - fixes servo#35833 - fixes servo#35982  - [x] There are new passing test: `css/css-logical/animation-004.html: Transitions from physical to logical update when the direction is changed` Created new test files as well: 1. `css-transitions/transition-remove-and-change-immediate.html` 2. `css-transitions/transition-zero-duration-with-delay.html` 3. `css-transitions/transitioncancel-003.html`   cc @Asun0204 @xiaochengh @stevennovaryo Signed-off-by: Euclid Ye <[email protected]>

….html` (#37179) Fix the mistake I made in #35978 Testing: No behaviour change. Signed-off-by: Euclid Ye <[email protected]>

yezhizhen mentioned this pull request Mar 14, 2025

Rework ElementAnimationSet::start_transition_if_applicable to be more like the specification servo/stylo#145

Merged

yezhizhen changed the title ~~Fix transition toggle & cancel~~ Fix transition toggle & cancellation & delay Mar 15, 2025

yezhizhen force-pushed the fix-transition branch from afb5c5a to 4a8883b Compare March 18, 2025 02:54

yezhizhen force-pushed the fix-transition branch from 4a8883b to 2541991 Compare March 18, 2025 09:34

yezhizhen force-pushed the fix-transition branch from 2541991 to a41a978 Compare March 19, 2025 04:34

servo-wpt-sync mentioned this pull request Mar 19, 2025

Fix transition toggle & cancellation & delay web-platform-tests/wpt#51439

Merged

yezhizhen force-pushed the fix-transition branch from a41a978 to 4168477 Compare March 19, 2025 05:46

yezhizhen force-pushed the fix-transition branch from 4168477 to 128a788 Compare March 19, 2025 05:54

yezhizhen force-pushed the fix-transition branch from 128a788 to 778c53d Compare March 19, 2025 06:03

yezhizhen force-pushed the fix-transition branch from 778c53d to 9bc85b9 Compare March 19, 2025 06:07

yezhizhen force-pushed the fix-transition branch from 9bc85b9 to fc3717c Compare March 19, 2025 06:37

sagudev requested a review from Loirooriol March 19, 2025 09:48

yezhizhen force-pushed the fix-transition branch from 92cbd5b to 60ccdc4 Compare March 20, 2025 06:37

Loirooriol reviewed Apr 4, 2025

View reviewed changes

yezhizhen requested a review from Loirooriol April 5, 2025 03:56

yezhizhen force-pushed the fix-transition branch from 0bdc692 to 5f61569 Compare April 5, 2025 03:58

yezhizhen force-pushed the fix-transition branch from 5f61569 to bb2837a Compare April 5, 2025 05:43

simonwuelker mentioned this pull request Apr 6, 2025

Unify the way html5ever and xml5ever block on script elements #36284

Merged

yezhizhen force-pushed the fix-transition branch from bb2837a to 568a10f Compare April 7, 2025 12:43

Loirooriol approved these changes Apr 7, 2025

View reviewed changes

Loirooriol enabled auto-merge April 7, 2025 12:45

Loirooriol force-pushed the fix-transition branch from 568a10f to 68cb614 Compare April 7, 2025 13:51

Fix transition toggle & cancel & delay

bd8d16d

Signed-off-by: Euclid Ye <[email protected]>

mrobinson force-pushed the fix-transition branch from 68cb614 to bd8d16d Compare April 7, 2025 14:18

Loirooriol added this pull request to the merge queue Apr 7, 2025

Merged via the queue into servo:main with commit 3242592 Apr 7, 2025
21 checks passed

yezhizhen deleted the fix-transition branch April 10, 2025 07:53

yezhizhen mentioned this pull request May 29, 2025

Chore: Remove unused variable in transition-zero-duration-with-delay.html #37179

Merged

github-merge-queue bot pushed a commit that referenced this pull request May 29, 2025

Chore: Remove unused variable in `transition-zero-duration-with-delay…

3bb7c71

….html` (#37179) Fix the mistake I made in #35978 Testing: No behaviour change. Signed-off-by: Euclid Ye <[email protected]>

Uh oh!

Conversation

yezhizhen commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yezhizhen commented Mar 18, 2025

Uh oh!

yezhizhen commented Mar 18, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 19, 2025

Uh oh!

servo-wpt-sync commented Mar 20, 2025

Uh oh!

servo-wpt-sync commented Mar 20, 2025

Uh oh!

servo-wpt-sync commented Mar 20, 2025

Uh oh!

servo-wpt-sync commented Mar 20, 2025

Uh oh!

yezhizhen commented Mar 20, 2025

Uh oh!

servo-wpt-sync commented Apr 4, 2025

Uh oh!

Loirooriol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Loirooriol Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

yezhizhen Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Loirooriol Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

yezhizhen Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

servo-wpt-sync commented Apr 5, 2025

Uh oh!

servo-wpt-sync commented Apr 5, 2025

Uh oh!

servo-wpt-sync commented Apr 5, 2025

Uh oh!

yezhizhen commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Loirooriol commented Apr 7, 2025

Uh oh!

yezhizhen commented Apr 7, 2025

Uh oh!

servo-wpt-sync commented Apr 7, 2025

yezhizhen commented Mar 14, 2025 •

edited

Loading

yezhizhen Apr 5, 2025 •

edited

Loading

yezhizhen Apr 5, 2025 •

edited

Loading

yezhizhen commented Apr 5, 2025 •

edited

Loading