Skip to content

script: Skip clearing subtree layout boxes of detached element when attaching shadow to it#44052

Merged
yezhizhen merged 3 commits into
servo:mainfrom
yezhizhen:skip-subtree
Apr 10, 2026
Merged

script: Skip clearing subtree layout boxes of detached element when attaching shadow to it#44052
yezhizhen merged 3 commits into
servo:mainfrom
yezhizhen:skip-subtree

Conversation

@yezhizhen

Copy link
Copy Markdown
Member

There is no layout boxes to clear in this case. For the example in #43998,
this skips the traversal many times.

Testing: Should not change visible behaviour. Try.

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen yezhizhen requested a review from xiaochengh April 9, 2026 01:43
@yezhizhen yezhizhen requested a review from gterzian as a code owner April 9, 2026 01:43
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 9, 2026
@yezhizhen yezhizhen changed the title script: Skip clearing layout boxes of detached elements script: Skip clearing layout boxes of detached elements when attaching shadow Apr 9, 2026
@yezhizhen yezhizhen changed the title script: Skip clearing layout boxes of detached elements when attaching shadow script: Skip clearing subtree layout boxes of detached element when attaching shadow to it Apr 9, 2026
@mrobinson

Copy link
Copy Markdown
Member

Do we clear layout boxes when a node is disconnected from the tree? I suspect we might not be doing that. This could lead to a situation where you attach a shadow to a removed node, put it back into the DOM, and then run into the same bug that #42237 was trying to fix.

.
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen

Copy link
Copy Markdown
Member Author

Do we clear layout boxes when a node is disconnected from the tree? I suspect we might not be doing that.

It seems every code path that disconnects a node guarantees layout boxes are cleared.

  • All removal eventually -> Node::remove -> remove_child -> complete_remove_subtree (clears style and layout)
  • All moves -> Node::move_child -> Node::complete_move_subtree (clears style and layout)

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 9, 2026
@yezhizhen yezhizhen added this pull request to the merge queue Apr 10, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 10, 2026
Merged via the queue into servo:main with commit 5604989 Apr 10, 2026
30 checks passed
@yezhizhen yezhizhen deleted the skip-subtree branch April 10, 2026 01:02
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants