Resolve inline containing blocks from first and last line rects - #11015
Conversation
|
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 (27)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (25)
📝 WalkthroughWalkthroughThe layout engine computes inline containing-block rectangles during inline layout, stores them in layout state, and uses them for absolute-positioned descendants. Tests cover atomic inlines, fragmented lines, writing modes, direction, scrolling, and dynamic positioning. ChangesInline containing-block positioning
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Rust/src/layout/abspos_engine.rs`:
- Around line 169-181: Add a short debug assertion before the ancestor walk in
the inline containing-block offset logic, using the existing callbacks/ancestor
relationship to verify that abspos_containing_block is an ancestor of
inline_node. Preserve the current walk for valid chains, but ensure the invalid
invariant is detected rather than returning an offset that bypasses the
containing block.
In
`@Tests/LibWeb/Ref/expected/wpt-import/css/css-position/position-absolute-in-inline-006-ref.html`:
- Line 5: Correct the malformed Ahem stylesheet URL in the three affected
reference files:
Tests/LibWeb/Ref/expected/wpt-import/css/css-position/position-absolute-in-inline-006-ref.html:5-5,
position-absolute-in-inline-010-ref.html:5-5, and
position-absolute-in-inline-011-ref.html:5-5. Replace the path containing the
four-dot component with the correct ../../../../fonts/ahem.css URL in each file.
🪄 Autofix
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: f2e72840-93a9-4bde-9e3f-d19e0b30ce31
📒 Files selected for processing (27)
Libraries/LibWeb/Rust/src/layout/abspos_engine.rsLibraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsLibraries/LibWeb/Rust/src/layout/line_box_fragment.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsTests/LibWeb/Layout/expected/abspos-inline-containing-block-atomic-inline-border-box.txtTests/LibWeb/Layout/expected/abspos-inline-containing-block-first-last-line-rule.txtTests/LibWeb/Layout/input/abspos-inline-containing-block-first-last-line-rule.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/hypothetical-box-scroll-parent-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/hypothetical-box-scroll-viewport-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/hypothetical-dynamic-change-001-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/position-absolute-in-inline-006-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/position-absolute-in-inline-010-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-position/position-absolute-in-inline-011-ref.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/hypothetical-box-scroll-parent.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/hypothetical-box-scroll-viewport.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/hypothetical-dynamic-change-001.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/hypothetical-dynamic-change-002.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/hypothetical-dynamic-change-003.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/nested-inline-abspos-child-with-siblings.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/nested-inline-abspos-child.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/position-absolute-in-inline-006.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/position-absolute-in-inline-010.htmlTests/LibWeb/Ref/input/wpt-import/css/css-position/position-absolute-in-inline-011.htmlTests/LibWeb/TestConfig.iniTests/LibWeb/Text/input/css/abspos-cb-inline-relative-with-block-descendant.html
💤 Files with no reviewable changes (2)
- Libraries/LibWeb/Rust/src/layout/line_builder.rs
- Libraries/LibWeb/Rust/src/layout/line_box_fragment.rs
Imports the css-position tests that exercise absolutely positioned boxes with inline containing blocks and hypothetical-box static positions. Four pass already; the six that target the containing block geometry of split and nested inlines fail against the current all-fragments union and are skipped with a note, to be unskipped by the switch to the first/last-line containing block rule.
When an absolutely positioned box's containing block is an inline box, the engine rebuilt the containing block rect at pass time by walking the box tree under the inline's non-anonymous containing block: a bounding box of every fragment on every line, border boxes of interrupting block descendants, and line-height-derived extents for atomic inlines, all read from line data and used values that other formatting contexts own. CSS2.1 section 10.1 defines the rect as the bounding box around the padding boxes of the first and the last inline boxes generated for the element: the inline's own fragments on the first and last content lines, with middle lines contributing nothing. A line generated by a block inside the inline participates through the inline box that wraps the block, so when such a line is first or last it carries the block's border box. The inline-box-pieces pass already tracks content lines with matching semantics, so compute() now derives the padding-box rect spanning the first and last content-line corners, contracting inline-axis borders only when the inline's direction matches its container's, and publishes it on the inline's used values when the owning inline formatting context finishes its lines. The consumer translates that one record along the containing block chain instead of walking the box tree; spans whose only content is interrupting blocks take the border box spanning those blocks, and fragment-less spans reuse the line-height placeholder. The pass-time walker and its cross-context line-data reads are deleted. This deliberately changes behavior to follow the specified rule: two previously failing WPT css-position imports now pass, the atomic-extent expectations are rebaselined, and a new layout test locks in first/last-line selection, direction-dependent border contraction, real atomic extents, a table-cell host, and a flex-registered child whose containing block is an outer inline. The still-skipped imports need multicol fragmentation, vertical-rl, all out of scope here.
5061269 to
379cfc0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
When an absolutely positioned box's containing block is an inline box,
the engine rebuilt the containing block rect at pass time by walking
the box tree under the inline's non-anonymous containing block: a
bounding box of every fragment on every line, border boxes of
interrupting block descendants, and line-height-derived extents for
atomic inlines, all read from line data and used values that other
formatting contexts own.
CSS2.1 section 10.1 defines the rect as the bounding box around the
padding boxes of the first and the last inline boxes generated for the
element: the inline's own fragments on the first and last content
lines, with middle lines contributing nothing. A line generated by a
block inside the inline participates through the inline box that wraps
the block, so when such a line is first or last it carries the block's
border box. The inline-box-pieces pass already tracks content lines
with matching semantics, so compute() now derives the padding-box rect
spanning the first and last content-line corners, contracting
inline-axis borders only when the inline's direction matches its
container's, and publishes it on the inline's used values when the
owning inline formatting context finishes its lines. The consumer
translates that one record along the containing block chain instead of
walking the box tree; spans whose only content is interrupting blocks
take the border box spanning those blocks, and fragment-less spans
reuse the line-height placeholder. The pass-time walker and its
cross-context line-data reads are deleted.
This deliberately changes behavior to follow the specified rule: two
previously failing WPT css-position imports now pass, the
atomic-extent expectations are rebaselined, and a new layout test
locks in first/last-line selection, direction-dependent border
contraction, real atomic extents, a table-cell host, and a
flex-registered child whose containing block is an outer inline. The
still-skipped imports need multicol fragmentation, vertical-rl, all out
of scope here.