LibWeb: Fix assorted style, layout, and painting issues - #10952
Merged
Conversation
Collaborator
|
Hello! One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds centralized container-query invalidation, updates pseudo-element content generation and layout snapshots, and adjusts CSS computed-style, rule-cache, transform, node identity, and SVG painting behavior. It also adds related test coverage. ChangesContainer Query Invalidation
Pseudo-element Layout
CSS and DOM State Handling
SVG Painting Order
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Libraries/LibWeb/DOM/Element.cpp (1)
1469-1484: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse flat-tree invalidation for style-container-query dependents.
for_each_shadow_including_descendantfollows DOM tree order, not flat-tree composition. A slotted style-dependent element can be a flat-tree descendant of the style query container while not being a shadow-including descendant of it, so its dependency can miss the invalidation path. Reuse the flat-tree child-walk approach used for size-container queries here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Element.cpp` around lines 1469 - 1484, Update Element::invalidate_descendant_styles_depending_on_style_container_query() to traverse descendants using the flat-tree child-walk mechanism, matching the approach used for size-container queries, instead of for_each_shadow_including_descendant. Preserve the existing style dependency check and set_needs_style_update behavior while ensuring slotted flat-tree descendants are included.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/Painting/StackingContext.cpp`:
- Around line 402-407: Preserve SVG tree-order painting for stacking-context
descendants. In Libraries/LibWeb/Painting/StackingContext.cpp lines 402-407,
replace the bulk post-traversal loop with logic that paints each child context
once at its document position; in Libraries/LibWeb/Painting/SVGSVGPaintable.cpp
lines 84-87, retain stacking-context children during traversal or provide an
equivalent atomic callback. Use the existing painting symbols and avoid painting
those children again after the SVG walk.
In `@Tests/LibWeb/Text/expected/css/quirks-mode-selector-case-insensitivity.txt`:
- Around line 1-9: Update the expected output for quirks-mode selector case
insensitivity to reflect the colors declared by the source fixture’s stylesheet,
rather than recording black for every selector. If the selector behavior is not
fixed in this change, remove this known-failing case from the passing baseline
instead; use the fixture `quirks-mode-selector-case-insensitivity` to locate the
relevant test.
---
Outside diff comments:
In `@Libraries/LibWeb/DOM/Element.cpp`:
- Around line 1469-1484: Update
Element::invalidate_descendant_styles_depending_on_style_container_query() to
traverse descendants using the flat-tree child-walk mechanism, matching the
approach used for size-container queries, instead of
for_each_shadow_including_descendant. Preserve the existing style dependency
check and set_needs_style_update behavior while ensuring slotted flat-tree
descendants are included.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e73702d9-fd69-40ee-b735-60e392d1b8bf
📒 Files selected for processing (47)
Libraries/LibWeb/CMakeLists.txtLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/ContainerQuery.cppLibraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.cppLibraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.hLibraries/LibWeb/CSS/Length.cppLibraries/LibWeb/CSS/StyleComputer.cppLibraries/LibWeb/CSS/StylePropertyMapReadOnly.cppLibraries/LibWeb/CSS/StyleScope.cppLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/DOM/Element.cppLibraries/LibWeb/DOM/Element.hLibraries/LibWeb/DOM/Node.cppLibraries/LibWeb/DOM/Node.hLibraries/LibWeb/Internals/Internals.cppLibraries/LibWeb/Layout/TreeBuilder.cppLibraries/LibWeb/Painting/Paintable.cppLibraries/LibWeb/Painting/SVGSVGPaintable.cppLibraries/LibWeb/Painting/StackingContext.cppLibraries/LibWeb/Rust/src/layout/tree_builder.rsTests/LibWeb/Layout/expected/abspos-pseudo-element-with-inline-as-abspos-containing-block.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-abspos-pseudo-element.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-after-pseudo.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-before-pseudo.txtTests/LibWeb/Layout/expected/block-and-inline/intrinsic-sizing-stress-test.txtTests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-inline-style.txtTests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-presentational-hints.txtTests/LibWeb/Layout/expected/empty-pseudo-element-content.txtTests/LibWeb/Layout/expected/grid/fit-content-3.txtTests/LibWeb/Layout/expected/pdf-viewer.txtTests/LibWeb/Layout/expected/position-empty-pseudo-elements.txtTests/LibWeb/Layout/expected/pseudo-element-with-custom-properties-2.txtTests/LibWeb/Layout/expected/pseudo-element-with-custom-properties.txtTests/LibWeb/Layout/expected/pseudo-element-with-display-block.txtTests/LibWeb/Layout/expected/table/abspos-pseudo-element-inside-table.txtTests/LibWeb/Layout/input/empty-pseudo-element-content.htmlTests/LibWeb/Text/expected/css/disconnected-node-mutation-no-style-invalidation.txtTests/LibWeb/Text/expected/css/quirks-mode-selector-case-insensitivity.txtTests/LibWeb/Text/expected/css/style-invalidation/size-query-container-scans.txtTests/LibWeb/Text/expected/css/style-invalidation/style-query-container-scans.txtTests/LibWeb/Text/expected/css/user-agent-sheet-media-rule-is-evaluated.txtTests/LibWeb/Text/input/css/disconnected-node-mutation-no-style-invalidation.htmlTests/LibWeb/Text/input/css/quirks-mode-selector-case-insensitivity.htmlTests/LibWeb/Text/input/css/style-invalidation/size-query-container-scans.htmlTests/LibWeb/Text/input/css/style-invalidation/style-query-container-scans.htmlTests/LibWeb/Text/input/css/user-agent-sheet-media-rule-is-evaluated.html
Comment on lines
+1
to
+9
| compatMode: BackCompat | ||
| class selector: rgb(0, 0, 0) | ||
| tag and class selector: rgb(0, 0, 0) | ||
| id selector: rgb(0, 0, 0) | ||
| tag and id selector: rgb(0, 0, 0) | ||
| descendant of class selector: rgb(0, 0, 0) | ||
| descendant of id selector: rgb(0, 0, 0) | ||
| class added later: rgb(0, 0, 0) | ||
| id added later: rgb(0, 0, 0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not encode the known failure as the regression baseline.
The source fixture Tests/LibWeb/Text/input/css/quirks-mode-selector-case-insensitivity.html, Lines 26-28, states that the black results are incorrect. The expected file nevertheless records black for every selector, so the test passes while the bug remains.
If this test verifies the fix, update Lines 2-9 to match the colors declared in the stylesheet or include the selector-matching fix in this PR. If this is intentionally a known-failing test, keep it out of the passing baseline.
Proposed expected-output update
compatMode: BackCompat
-class selector: rgb(0, 0, 0)
+class selector: rgb(0, 128, 0)
-tag and class selector: rgb(0, 0, 0)
+tag and class selector: rgb(0, 0, 255)
-id selector: rgb(0, 0, 0)
+id selector: rgb(255, 0, 0)
-tag and id selector: rgb(0, 0, 0)
+tag and id selector: rgb(255, 0, 255)
-descendant of class selector: rgb(0, 0, 0)
+descendant of class selector: rgb(0, 128, 128)
-descendant of id selector: rgb(0, 0, 0)
+descendant of id selector: rgb(128, 0, 128)
-class added later: rgb(0, 0, 0)
+class added later: rgb(128, 128, 0)
-id added later: rgb(0, 0, 0)
+id added later: rgb(255, 128, 0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| compatMode: BackCompat | |
| class selector: rgb(0, 0, 0) | |
| tag and class selector: rgb(0, 0, 0) | |
| id selector: rgb(0, 0, 0) | |
| tag and id selector: rgb(0, 0, 0) | |
| descendant of class selector: rgb(0, 0, 0) | |
| descendant of id selector: rgb(0, 0, 0) | |
| class added later: rgb(0, 0, 0) | |
| id added later: rgb(0, 0, 0) | |
| compatMode: BackCompat | |
| class selector: rgb(0, 128, 0) | |
| tag and class selector: rgb(0, 0, 255) | |
| id selector: rgb(255, 0, 0) | |
| tag and id selector: rgb(255, 0, 255) | |
| descendant of class selector: rgb(0, 128, 128) | |
| descendant of id selector: rgb(128, 0, 128) | |
| class added later: rgb(128, 128, 0) | |
| id added later: rgb(255, 128, 0) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/LibWeb/Text/expected/css/quirks-mode-selector-case-insensitivity.txt`
around lines 1 - 9, Update the expected output for quirks-mode selector case
insensitivity to reflect the colors declared by the source fixture’s stylesheet,
rather than recording black for every selector. If the selector behavior is not
fixed in this change, remove this known-failing case from the passing baseline
instead; use the fixture `quirks-mode-selector-case-insensitivity` to locate the
relevant test.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Libraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.cpp (1)
40-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated traversal loop.
invalidate_descendant_styles_depending_on_size_container_queryandinvalidate_descendant_styles_depending_on_style_container_queryrun the same stack-based flat-tree DFS. The only differences are the container-type gate, the counter increment, and which dependency flag gets checked. The counter placement has already diverged between the two copies (per-visit for size, per-scan for style), which is the kind of drift that duplicated loops invite. Extract the loop into one helper that both functions call with a small callback, so future changes to the traversal apply to both scans automatically.♻️ Proposed refactor to share the traversal loop
+template<typename Callback> +static void for_each_flat_tree_descendant_element(DOM::Element& query_container, Callback&& callback) +{ + Vector<DOM::Node*> stack; + append_flat_tree_children(query_container, stack); + while (!stack.is_empty()) { + auto* node = stack.take_last(); + if (auto* element = as_if<DOM::Element>(*node)) + callback(*element); + append_flat_tree_children(*node, stack); + } +} + void invalidate_descendant_styles_depending_on_size_container_query(DOM::Element& query_container) { if (!query_container.is_size_query_container()) return; auto& counters = query_container.document().style_invalidation_counters(); - Vector<DOM::Node*> stack; - append_flat_tree_children(query_container, stack); - while (!stack.is_empty()) { - auto* node = stack.take_last(); - if (auto* element = as_if<DOM::Element>(*node)) { - ++counters.size_query_container_scan_visits; - if (element->style_depends_on_size_container_query()) - element->set_needs_style_update(true); - } - append_flat_tree_children(*node, stack); - } + for_each_flat_tree_descendant_element(query_container, [&](DOM::Element& element) { + ++counters.size_query_container_scan_visits; + if (element.style_depends_on_size_container_query()) + element.set_needs_style_update(true); + }); } void invalidate_descendant_styles_depending_on_style_container_query(DOM::Element& query_container) { if (!query_container.is_style_query_container()) return; ++query_container.document().style_invalidation_counters().style_query_container_scans; - Vector<DOM::Node*> stack; - append_flat_tree_children(query_container, stack); - while (!stack.is_empty()) { - auto* node = stack.take_last(); - if (auto* element = as_if<DOM::Element>(*node)) { - if (element->style_depends_on_style_container_query()) - element->set_needs_style_update(true); - } - append_flat_tree_children(*node, stack); - } + for_each_flat_tree_descendant_element(query_container, [](DOM::Element& element) { + if (element.style_depends_on_style_container_query()) + element.set_needs_style_update(true); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.cpp` around lines 40 - 81, Extract the shared stack-based flat-tree DFS from invalidate_descendant_styles_depending_on_size_container_query and invalidate_descendant_styles_depending_on_style_container_query into a helper accepting a callback for per-element dependency handling. Keep each function’s existing container-type guard and counter placement unchanged, and have the callbacks perform their respective dependency checks and style-update marking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Libraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.cpp`:
- Around line 40-81: Extract the shared stack-based flat-tree DFS from
invalidate_descendant_styles_depending_on_size_container_query and
invalidate_descendant_styles_depending_on_style_container_query into a helper
accepting a callback for per-element dependency handling. Keep each function’s
existing container-type guard and counter placement unchanged, and have the
callbacks perform their respective dependency checks and style-update marking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 79eced1b-4da1-4b7d-85c6-204434872288
📒 Files selected for processing (45)
Libraries/LibWeb/CMakeLists.txtLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/ContainerQuery.cppLibraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.cppLibraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.hLibraries/LibWeb/CSS/Length.cppLibraries/LibWeb/CSS/StyleComputer.cppLibraries/LibWeb/CSS/StylePropertyMapReadOnly.cppLibraries/LibWeb/CSS/StyleScope.cppLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/DOM/Element.cppLibraries/LibWeb/DOM/Element.hLibraries/LibWeb/DOM/Node.cppLibraries/LibWeb/DOM/Node.hLibraries/LibWeb/Internals/Internals.cppLibraries/LibWeb/Layout/TreeBuilder.cppLibraries/LibWeb/Painting/Paintable.cppLibraries/LibWeb/Painting/SVGSVGPaintable.cppLibraries/LibWeb/Painting/StackingContext.hLibraries/LibWeb/Rust/src/layout/tree_builder.rsTests/LibWeb/Layout/expected/abspos-pseudo-element-with-inline-as-abspos-containing-block.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-abspos-pseudo-element.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-after-pseudo.txtTests/LibWeb/Layout/expected/block-and-inline/button-with-before-pseudo.txtTests/LibWeb/Layout/expected/block-and-inline/intrinsic-sizing-stress-test.txtTests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-inline-style.txtTests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-presentational-hints.txtTests/LibWeb/Layout/expected/empty-pseudo-element-content.txtTests/LibWeb/Layout/expected/grid/fit-content-3.txtTests/LibWeb/Layout/expected/pdf-viewer.txtTests/LibWeb/Layout/expected/position-empty-pseudo-elements.txtTests/LibWeb/Layout/expected/pseudo-element-with-custom-properties-2.txtTests/LibWeb/Layout/expected/pseudo-element-with-custom-properties.txtTests/LibWeb/Layout/expected/pseudo-element-with-display-block.txtTests/LibWeb/Layout/expected/table/abspos-pseudo-element-inside-table.txtTests/LibWeb/Layout/input/empty-pseudo-element-content.htmlTests/LibWeb/Text/expected/css/disconnected-node-mutation-no-style-invalidation.txtTests/LibWeb/Text/expected/css/style-invalidation/size-query-container-scans.txtTests/LibWeb/Text/expected/css/style-invalidation/style-query-container-scans.txtTests/LibWeb/Text/expected/css/user-agent-sheet-media-rule-is-evaluated.txtTests/LibWeb/Text/input/css/disconnected-node-mutation-no-style-invalidation.htmlTests/LibWeb/Text/input/css/style-invalidation/size-query-container-scans.htmlTests/LibWeb/Text/input/css/style-invalidation/style-query-container-scans.htmlTests/LibWeb/Text/input/css/user-agent-sheet-media-rule-is-evaluated.html
🚧 Files skipped from review as they are similar to previous changes (42)
- Libraries/LibWeb/Rust/src/layout/tree_builder.rs
- Libraries/LibWeb/CSS/StyleScope.cpp
- Libraries/LibWeb/CMakeLists.txt
- Libraries/LibWeb/CSS/StyleComputer.cpp
- Tests/LibWeb/Text/expected/css/user-agent-sheet-media-rule-is-evaluated.txt
- Tests/LibWeb/Text/input/css/disconnected-node-mutation-no-style-invalidation.html
- Libraries/LibWeb/CSS/ComputedValues.h
- Tests/LibWeb/Layout/expected/position-empty-pseudo-elements.txt
- Tests/LibWeb/Layout/expected/block-and-inline/button-with-after-pseudo.txt
- Libraries/LibWeb/CSS/ContainerQuery.cpp
- Libraries/LibWeb/CSS/Invalidation/ContainerQueryInvalidator.h
- Tests/LibWeb/Layout/expected/block-and-inline/button-with-before-pseudo.txt
- Libraries/LibWeb/CSS/Length.cpp
- Tests/LibWeb/Layout/expected/grid/fit-content-3.txt
- Libraries/LibWeb/DOM/Document.h
- Tests/LibWeb/Text/input/css/user-agent-sheet-media-rule-is-evaluated.html
- Tests/LibWeb/Layout/expected/table/abspos-pseudo-element-inside-table.txt
- Tests/LibWeb/Text/input/css/style-invalidation/style-query-container-scans.html
- Tests/LibWeb/Layout/expected/abspos-pseudo-element-with-inline-as-abspos-containing-block.txt
- Libraries/LibWeb/Internals/Internals.cpp
- Tests/LibWeb/Layout/expected/pseudo-element-with-display-block.txt
- Tests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-inline-style.txt
- Libraries/LibWeb/Painting/SVGSVGPaintable.cpp
- Tests/LibWeb/Text/input/css/style-invalidation/size-query-container-scans.html
- Libraries/LibWeb/Painting/Paintable.cpp
- Libraries/LibWeb/Layout/TreeBuilder.cpp
- Tests/LibWeb/Text/expected/css/style-invalidation/style-query-container-scans.txt
- Tests/LibWeb/Text/expected/css/disconnected-node-mutation-no-style-invalidation.txt
- Tests/LibWeb/Text/expected/css/style-invalidation/size-query-container-scans.txt
- Libraries/LibWeb/DOM/Element.h
- Tests/LibWeb/Layout/expected/block-and-inline/intrinsic-sizing-stress-test.txt
- Tests/LibWeb/Layout/expected/css-pseudo-element-should-not-be-affected-by-presentational-hints.txt
- Libraries/LibWeb/DOM/Document.cpp
- Tests/LibWeb/Layout/expected/pdf-viewer.txt
- Tests/LibWeb/Layout/input/empty-pseudo-element-content.html
- Tests/LibWeb/Layout/expected/block-and-inline/button-with-abspos-pseudo-element.txt
- Libraries/LibWeb/DOM/Node.cpp
- Tests/LibWeb/Layout/expected/pseudo-element-with-custom-properties-2.txt
- Libraries/LibWeb/DOM/Node.h
- Tests/LibWeb/Layout/expected/pseudo-element-with-custom-properties.txt
- Tests/LibWeb/Layout/expected/empty-pseudo-element-content.txt
- Libraries/LibWeb/CSS/StylePropertyMapReadOnly.cpp
awesomekling
force-pushed
the
misc-fixes
branch
3 times, most recently
from
July 31, 2026 15:18
b2aa3ff to
2eb84bb
Compare
awesomekling
force-pushed
the
misc-fixes
branch
2 times, most recently
from
August 9, 2026 10:07
e37cfe5 to
f39d9e9
Compare
github-actions
Bot
added
the
conflicts
|
Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest |
A paintable outlives the layout node it was made for, and a style recompute can reach an element whose layout node is already gone: the layout tree builder updates the style of an element a bypass path reached, and `display: none` leaves the flag set until then. Both the transition and the animation setup asked such a paintable for its transform reference box, which reads the layout node and fails the verification in `Paintable::layout_node()`. A reference box is a fact about a layout box, so without one there is none, exactly as when the element was never painted at all. This is reached once StyleGraph owns structural invalidation, where `child-state-pseudo-invalidation` crashes on it.
The same bug as the reset group's style value: `list_style_image` is a `RefPtr` compared by a defaulted `operator==`, so a recomputed marker image reports the group different when the list style has not moved. No test measures a change from this - nothing in the suite recomputes an element carrying a marker image - but the comparison is wrong either way, and it is the shape every remaining group needs.
Every element style change walked the element's whole shadow-including subtree looking for descendants whose style container queries had to be re-evaluated. Nothing bounded that walk: a document with no @container style() rule at all paid it once per changed element, and a deep subtree paid it in full. A style container query resolves against exactly one ancestor, and that ancestor is known when the query is evaluated, so it is marked there and the scan runs only for an element some query actually selected. The value the query compares against is resolved as well, and that resolution can read the root, so the root is marked too - a `calc(1rem * 10)` in the query moves when the root font size does. styleQueryContainerScans counts the scans that survive, and the test added with it pins both halves: no scan for a change on an element no query names, and the dependent still updating when the container changes.
A query container whose size moved scanned its whole subtree for the elements whose size queries and container-relative units resolved against it. `container-type` is set far more widely than it is asked about, and the scan ran after every layout that moved such a box, so a design system that declares `container-type: inline-size` broadly paid for a subtree walk per container per layout with nothing under it to find. The container a query resolves against is known when the query is evaluated, so it is marked there - by a size feature in an `@container` condition and by a `cq*` unit alike - and the scan runs only for a container something named. The walk follows the flat tree now, which is the inverse of the walk that selects a container: a slotted element's container can be inside the shadow tree it is slotted into, and a DOM walk from that container never reaches it. That case was only reached before because a container further out was being scanned unconditionally as well. sizeQueryContainerScanVisits counts the elements the scans look at.
A user-agent stylesheet is a process-wide singleton with no owning document, so the walk that evaluates a document's media rules never reaches one. Its `@media` answers are still per document, though: `(scripting)` is false in a document with no browsing context and true in an ordinary one. The rule cache reads the answer that happens to be recorded on the shared rule, so which document last asked decided whether `noscript` got the user-agent sheet's `display: none`, and on en.wikipedia.org/wiki/CSS it did not. Evaluate the user-agent and user sheets against the document where the rule cache that consumes the answer is built, which is the one place that cannot be reached out of order. Both of those sheets' conditions are fixed for a document's lifetime, so nothing has to invalidate on a later flip. The test pins both answers: the document's own `noscript` is hidden, and a scripting-disabled frame's is not.
An `<svg>` that establishes a stacking context painted its SVG descendants through the SVG walk and returned, so any descendant with a stacking context of its own was never painted at all. A `<foreignObject>` always has one, so a standalone SVG whose root has a stacking context rendered nothing inside its foreign objects. The two paths were also drawing such a descendant twice where the root was not a stacking context: the SVG walk painted it and its own context painted it again. The walk skips it now, the way the general descendant walk already does. This surfaces once `:root` matches the root of a standalone SVG, which gives it `view-transition-name: root` from the user-agent sheet.
`getComputedStyle` refuses a disconnected element outright, but the computed style property map answered from whatever the rule caches matched against the detached tree, so a class set on a detached ancestor still decided a colour. No other engine does that: Chrome reports size 0, `has` false and `get` undefined for every property. The map now answers the same way. The disconnected-mutation test carried the old answer and now records this one, with the size and `has` checks added beside it.
Empty generated strings only need a layout node when they carry the inline fragment of an ordinary inline pseudo-element. Creating a GeneratedTextNode inside every other pseudo box made otherwise empty inline-block, block, flex, and grid boxes take the non-empty intrinsic sizing path. Allow content-item construction to produce no layout node for those empty strings. Preserve the node for ordinary inline pseudo-elements, where it still carries padding, borders, and other inline geometry. Add layout coverage for both cases and update affected tree dumps.
Allocate and register a node identifier on its first use instead of for every constructed node. Most nodes never expose their identifier to an external consumer, so eager registration only adds weak hash map work to DOM construction. Only deregister nodes that were assigned an identifier.
awesomekling
force-pushed
the
misc-fixes
branch
from
August 9, 2026 10:58
f39d9e9 to
251fb1a
Compare
github-actions
Bot
removed
the
conflicts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch collects various LibWeb correctness fixes that I accumulated while investigating StyleBench: