Box tree construction: only unset pseudo elements if the box is rebuilt by longvatrong111 · Pull Request #39930 · servo/servo

longvatrong111 · 2025-10-16T19:32:02Z

Box Tree re-construction may cause the loss of pseudo element: #39225 (comment), then causes transition stucks.
We should only unset pseudo elements if the box is rebuilt.

Testing: fix manual test in #39225, CI reports no regression: action
Fixes: #39225

cc: @xiaochengh

jdm · 2025-10-16T20:15:34Z

Can you write an automated test for this?

servo-wpt-sync · 2025-10-20T08:33:23Z

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

servo-wpt-sync · 2025-10-20T08:36:40Z

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

servo-wpt-sync · 2025-10-20T08:57:38Z

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

yezhizhen

It should be possible to write a test without testdriver.js. We want an action that triggers reflow but not recollection, which can be done by style change.
I haven't checked this part of code for a while, what do you think? @mrobinson

longvatrong111 · 2025-10-21T06:49:01Z

It should be possible to write a test without testdriver.js. We want an action that triggers reflow but not recollection, which can be done by style change. I haven't checked this part of code for a while, what do you think? @mrobinson

I also think it's better if we don't need to use testdriver. However, right now I only know this way to reproduce the issue.
We need an action that rebuilds the box tree, not re-collect children, but reuse the host box of pseudo element. Changing style would force rebuilding the box so pseudo element is re-built as well and nothing happens.

yezhizhen

https://github.com/web-platform-tests/wpt/pull/55540/checks?check_run_id=53158603955

It seems the testdriver test-case provided has very specific coordinates and only work in Servo, failing all other browsers.. I don't quite understand how is your coordinates determined.

stevennovaryo

I am afraid the test with Pointer might be too unstable and complex, primarily due to Webdriver implementations and difference in reflowing. We might need a to look why it is happening in the first place and more general case.

stevennovaryo · 2025-10-20T08:59:21Z

...sts/css/css-transitions/pseudo-element-transition-continues-on-box-tree-re-construction.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>::before transition keeps updating while pointer moves to another element (IntersectionObserver)</title>
+<link rel="help" href="https://drafts.csswg.org/css-transitions/">


Suggested change

<link rel="help" href="https://drafts.csswg.org/css-transitions/">

<link rel="author" title="<your name>" href="mailto:<your email>@gmail.com">

Please add the contact.

stevennovaryo · 2025-10-20T09:24:03Z

...sts/css/css-transitions/pseudo-element-transition-continues-on-box-tree-re-construction.html

+<title>::before transition keeps updating while pointer moves to another element (IntersectionObserver)</title>
+<link rel="help" href="https://drafts.csswg.org/css-transitions/">
+<link rel="help" href="https://w3c.github.io/IntersectionObserver/">
+<meta name="assert" content="Moving pointer to another element does not freeze a ::before width transition on the host; host keeps expanding until it intersects the other element.">


Suggested change

<meta name="assert" content="Moving pointer to another element does not freeze a ::before width transition on the host; host keeps expanding until it intersects the other element.">

<link rel="help" href="https://github.com/servo/servo/issues/39225"/>

You could also add the issue reference.

Thank you, I updated.

stevennovaryo · 2025-10-21T08:41:39Z

I am wondering whether we have unit tests for incremental layout. If not, whether it is plausible to introduce it or not?

yezhizhen · 2025-10-21T08:48:22Z

Or, @longvatrong111 can you move the test to the folder which won't export to upstream?

This might be vulnerable to future UA stylesheet change tho..

mrobinson · 2025-10-21T11:00:07Z

I also think it's better if we don't need to use testdriver. However, right now I only know this way to reproduce the issue. We need an action that rebuilds the box tree, not re-collect children, but reuse the host box of pseudo element. Changing style would force rebuilding the box so pseudo element is re-built as well and nothing happens.

Only certain style changes cause a rebuilding of the box tree. You can see what kind of damage style changes inflict here: https://github.com/servo/stylo/blob/main/style/servo/restyle_damage.rs#L152.

coding-joedow · 2025-10-21T15:54:23Z

components/layout/dom_traversal.rs

 ) {
-    element.unset_all_pseudo_boxes();
+    let damage = element.take_restyle_damage();
+    if damage.contains(LayoutDamage::recollect_box_tree_children().into()) {


Suggested change

if damage.contains(LayoutDamage::recollect_box_tree_children().into()) {

if damage.has_box_damage() {

I think it's reasonable.

longvatrong111 · 2025-10-22T06:56:51Z

I also think it's better if we don't need to use testdriver. However, right now I only know this way to reproduce the issue. We need an action that rebuilds the box tree, not re-collect children, but reuse the host box of pseudo element. Changing style would force rebuilding the box so pseudo element is re-built as well and nothing happens.

Only certain style changes cause a rebuilding of the box tree. You can see what kind of damage style changes inflict here: https://github.com/servo/stylo/blob/main/style/servo/restyle_damage.rs#L152.

Interesting, I tried all 4 levels of style change but still can't reproduce the issue...

servo-wpt-sync · 2025-10-22T07:24:12Z

🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#55540).

Signed-off-by: batu_hoang <[email protected]>

longvatrong111 · 2025-10-22T07:37:04Z

https://github.com/web-platform-tests/wpt/pull/55540/checks?check_run_id=53158603955

It seems the testdriver test-case provided has very specific coordinates and only work in Servo, failing all other browsers.. I don't quite understand how is your coordinates determined.

I am afraid the test with Pointer might be too unstable and complex, primarily due to Webdriver implementations and difference in reflowing. We might need a to look why it is happening in the first place and more general case.

Seem like using pointer is the only way now.
The test expectation is unchanged: we expect transition to continue to regardless of other actions (unless the action implies stopping transition). The actions could be style changes or mouse move,...
So no need to worry about the correctness, the remaining task is trying to create the conditions to reproduce the issue. Using testdriver is 1 option.

Or, @longvatrong111 can you move the test to the folder which won't export to upstream?

It's reasonable.

Signed-off-by: batu_hoang <[email protected]>

yezhizhen

I'm suspicious of the test. I feel the stylesheet is overly complicated and not minimal. But since it is only local test and not running in CI yet, it shouldn't be a big issue and we can always change later.

The fix itself is reasonable and minimal.

yezhizhen · 2025-11-04T03:31:16Z

@longvatrong111 It seems this test is stably failing now, but the original issue is already gone.
Can you fix the test?

longvatrong111 requested review from Loirooriol and mrobinson as code owners October 16, 2025 19:32

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 16, 2025

longvatrong111 requested review from mrobinson, stevennovaryo and xiaochengh and removed request for mrobinson October 16, 2025 19:32

longvatrong111 changed the title ~~Box tree construction: only unset pseudo elements if the box is rebuit~~ Box tree construction: only unset pseudo elements if the box is rebuilt Oct 17, 2025

longvatrong111 force-pushed the pseudo-element-incremental-layout branch from cacab52 to 1cc1b12 Compare October 20, 2025 08:30

longvatrong111 requested a review from yezhizhen October 20, 2025 08:30

servo-wpt-sync mentioned this pull request Oct 20, 2025

Box tree construction: only unset pseudo elements if the box is rebuilt web-platform-tests/wpt#55540

Closed

longvatrong111 force-pushed the pseudo-element-incremental-layout branch from 1cc1b12 to a5cda0d Compare October 20, 2025 08:52

yezhizhen reviewed Oct 20, 2025

View reviewed changes

yezhizhen reviewed Oct 21, 2025

View reviewed changes

stevennovaryo suggested changes Oct 21, 2025

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 21, 2025

coding-joedow reviewed Oct 21, 2025

View reviewed changes

longvatrong111 force-pushed the pseudo-element-incremental-layout branch from a5cda0d to 03992ad Compare October 22, 2025 07:23

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Oct 22, 2025

Box tree construction: only unset pseudo elements if the box is rebuilt

a508c61

Signed-off-by: batu_hoang <[email protected]>

longvatrong111 force-pushed the pseudo-element-incremental-layout branch 2 times, most recently from f540159 to 8a582eb Compare October 22, 2025 07:27

longvatrong111 force-pushed the pseudo-element-incremental-layout branch from 8a582eb to c91c426 Compare October 22, 2025 08:05

Add WPT test for transition of pseudo element

2fd5580

Signed-off-by: batu_hoang <[email protected]>

longvatrong111 force-pushed the pseudo-element-incremental-layout branch from c91c426 to 2fd5580 Compare October 22, 2025 08:31

longvatrong111 requested review from coding-joedow, stevennovaryo and yezhizhen October 22, 2025 09:17

yezhizhen approved these changes Oct 23, 2025

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 23, 2025

yezhizhen added this pull request to the merge queue Oct 23, 2025

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 23, 2025

Merged via the queue into servo:main with commit 8132808 Oct 23, 2025
30 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 23, 2025

longvatrong111 deleted the pseudo-element-incremental-layout branch January 9, 2026 06:20

	<link rel="help" href="https://drafts.csswg.org/css-transitions/">
	<link rel="author" title="<your name>" href="mailto:<your email>@gmail.com">

	<meta name="assert" content="Moving pointer to another element does not freeze a ::before width transition on the host; host keeps expanding until it intersects the other element.">
	<link rel="help" href="https://github.com/servo/servo/issues/39225"/>

	if damage.contains(LayoutDamage::recollect_box_tree_children().into()) {
	if damage.has_box_damage() {

Uh oh!

Conversation

longvatrong111 commented Oct 16, 2025 • edited by mrobinson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdm commented Oct 16, 2025

Uh oh!

servo-wpt-sync commented Oct 20, 2025

Uh oh!

servo-wpt-sync commented Oct 20, 2025

Uh oh!

servo-wpt-sync commented Oct 20, 2025

Uh oh!

yezhizhen left a comment • 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!

longvatrong111 commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yezhizhen left a comment • 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!

stevennovaryo left a comment

Choose a reason for hiding this comment

Uh oh!

stevennovaryo Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

stevennovaryo Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

longvatrong111 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

stevennovaryo commented Oct 21, 2025

Uh oh!

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

Uh oh!

Uh oh!

mrobinson commented Oct 21, 2025

Uh oh!

coding-joedow Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

longvatrong111 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

longvatrong111 commented Oct 22, 2025

Uh oh!

servo-wpt-sync commented Oct 22, 2025

Uh oh!

longvatrong111 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yezhizhen left a comment • 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!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

longvatrong111 commented Oct 16, 2025 •

edited by mrobinson

Loading

yezhizhen left a comment •

edited

Loading

longvatrong111 commented Oct 21, 2025 •

edited

Loading

yezhizhen left a comment •

edited

Loading

yezhizhen commented Oct 21, 2025 •

edited

Loading

longvatrong111 commented Oct 22, 2025 •

edited

Loading

yezhizhen left a comment •

edited

Loading

yezhizhen commented Nov 4, 2025 •

edited

Loading