Skip to content

Don't compute style for children of shadow hosts#35198

Merged
simonwuelker merged 1 commit intoservo:mainfrom
simonwuelker:no-style-for-shadow-host-children
Jan 28, 2025
Merged

Don't compute style for children of shadow hosts#35198
simonwuelker merged 1 commit intoservo:mainfrom
simonwuelker:no-style-for-shadow-host-children

Conversation

@simonwuelker
Copy link
Copy Markdown
Member

@simonwuelker simonwuelker commented Jan 28, 2025

There is some interesting history to this change:

Now, we compute style for both the children of the element and a potential shadow root, but only lay out the contents of the shadow tree (if there is one).

This behaviour is not technically incorrect, since regular children are not included in layout their style doesn't matter. However, it is inefficient: the only case where we need to compute style for a child of a shadow host is when that child is an assigned slottable in a slot somewhere else.

This part 1/n of upstreaming the changes necessary to lay out <slot> contents. Note that trying to compute style for contents and children of shadow hosts will crash in stylo, since it expects to see eachelement only once.

try run


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes are a part of Support ShadowDom #34318
  • There are tests for these changes (covered by wpt)

@simonwuelker simonwuelker changed the title Never compute style for children of shadow hosts Don't compute style for children of shadow hosts Jan 28, 2025
@simonwuelker simonwuelker added this pull request to the merge queue Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
There is some interesting history to this change:

* #33495: Servo crashes on
  Cloudflare's turnstile, because we didn't compute style for
  elements inside shadow trees
* #34298: Resolves the issue
  by computing style for children of a potential shadow root,
  in addition to the children of an element
* #34701: Changes layout
  of elements with shadow roots such that only the contents
  of the shadow root are laid out

Now, we compute style for both the children of the element
and a potential shadow root, but only lay out the contents
of the shadow tree (if there is one).

This behaviour is not technically incorrect,
since regular children are not included in layout
their style doesn't matter. However, it is
inefficient: the only case where we need to compute
style for a child of a shadow host is when
that child is an assigned slottable in a slot
somewhere else.

This part 1/n of upstreaming the changes necessary
to lay out <slot> contents. Note that trying to compute
style for <slot> contents *and* children of shadow hosts
will crash in stylo, since it expects to see each
element only once.

Signed-off-by: Simon Wülker <[email protected]>
@simonwuelker simonwuelker force-pushed the no-style-for-shadow-host-children branch from bf048b2 to e92c9e1 Compare January 28, 2025 16:20
auto-merge was automatically disabled January 28, 2025 16:24

Pull Request is not mergeable

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 28, 2025
There is some interesting history to this change:

* servo#33495: Servo crashes on
  Cloudflare's turnstile, because we didn't compute style for
  elements inside shadow trees
* servo#34298: Resolves the issue
  by computing style for children of a potential shadow root,
  in addition to the children of an element
* servo#34701: Changes layout
  of elements with shadow roots such that only the contents
  of the shadow root are laid out

Now, we compute style for both the children of the element
and a potential shadow root, but only lay out the contents
of the shadow tree (if there is one).

This behaviour is not technically incorrect,
since regular children are not included in layout
their style doesn't matter. However, it is
inefficient: the only case where we need to compute
style for a child of a shadow host is when
that child is an assigned slottable in a slot
somewhere else.

This part 1/n of upstreaming the changes necessary
to lay out `<slot>` contents. Note that trying to compute
style for `<slot>` contents *and* children of shadow hosts
will crash in stylo, since it expects to see each
element only once.

Signed-off-by: Simon Wülker <[email protected]>
@simonwuelker simonwuelker force-pushed the no-style-for-shadow-host-children branch from e92c9e1 to 9126e81 Compare January 28, 2025 20:25
@simonwuelker simonwuelker added this pull request to the merge queue Jan 28, 2025
Merged via the queue into servo:main with commit 1188d2b Jan 28, 2025
@simonwuelker simonwuelker deleted the no-style-for-shadow-host-children branch January 28, 2025 21:36
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.

2 participants