Place every box before its formatting context returns - #11044
Merged
kalenikaliaksandr merged 4 commits intoAug 7, 2026
Merged
Conversation
We want a formatting context run to place every box it is responsible for before run() returns. Floats violated this: the BFC accumulated FloatingBox records during the run, and run_formatting_context placed them via place_floats_after_run() only after run() had returned. The deferral no longer serves a purpose: every input to the deferred math is fixed once the record is registered, and the stored offsets are containing-block-relative, so later subtree translations move the containing block and the float together. Place the float at the tail of layout_floating_box, right after the last write to its sealed metrics, and delete place_floats_after_run(). Since placement seals the float's metrics, a post-registration write now trips an assert instead of silently changing geometry. No behavior change.
The inline-level iterator entered <br> through the box-model path, so every break node acquired used values that nothing ever placed. They were pure iterator scratch: a break node produces no line-box fragment and no paintable, so the metrics computed for it at commit were discarded without reaching the C++ side. A <br> is a line-break control item, not a box. Skip it in the iterator's box-model enter path so it never gets used values; an overlooked reader now fails loudly on the missing-used-values assert. Floating and absolutely positioned <br> are unaffected. Box-model properties on a <br> no longer displace the first item of the following line; the one affected expectation (a universal selector put a 1px border on the <br>) is rebaselined.
A layout pass creates used values for its root boxes before any formatting context runs, so no run is responsible for placing them: the viewport and a standalone SVG document root reached commit with has_content_offset unset, relying on default-initialized (0,0) offsets. Subtree relayout already places its root when materializing it from the previous paintable; extend that rule to every pass entry. The full-layout entry now places its run root right after run_formatting_context returns (the run still finalizes the root's used sizes, so placement cannot happen earlier), and before the abspos pass so the seal covers it. The viewport in the SVG-document case and the subtree-relayout auxiliary viewport have final metrics at creation and are placed on the spot. Placement seals the roots' committed metrics, so a stray late write to viewport geometry now trips an assert instead of passing silently. No behavior change.
foreignObject content is rendered under the CSS model: an SVG element inside it has no SVG rendering context until a nested <svg> establishes a new one. The tree builder only tracked entering <svg> roots, so such elements got layout boxes anyway; a block-level sibling then made the BFC treat one as a block-level child, create used values for it, and bail on the not-a-block-container path without ever placing it — tripping the every-created-box-is-placed placement audit. The shape is reachable via XML parsing or createElementNS (in HTML parsing, foreignObject is an integration point). Clear the SVG-context flag when entering a foreignObject, so its SVG-namespace descendants are skipped by the existing suppression for SVG elements outside an <svg> root. Referenced mask and clip-path subtrees are SVG content in their own right wherever the referencing element sits, so the resource walk now establishes SVG context explicitly; a masked foreignObject previously relied on the leaked flag.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe layout tree builder now handles SVG ChangesLayout updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LayoutTreeBuilder
participant FfiPrincipalNodeEntryFacts
participant RustTreeBuilder
participant SvgTraversalState
LayoutTreeBuilder->>FfiPrincipalNodeEntryFacts: provide is_svg_foreign_object
FfiPrincipalNodeEntryFacts->>RustTreeBuilder: evaluate SVG entry decision
RustTreeBuilder->>SvgTraversalState: clear SVG-root state for EnterForeignContent
SvgTraversalState-->>RustTreeBuilder: restore prior state after traversal
Possibly related PRs
Suggested reviewers: 🚥 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 |
kalenikaliaksandr
enabled auto-merge (rebase)
August 7, 2026 21:02
kalenikaliaksandr
disabled auto-merge
August 7, 2026 21:03
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
1 participant
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 enforces new layout invariant: a formatting context run must place every box it's responsible for before its run() returns and every box that has used values has to be placed by the time layout commits.