LibWeb: Move list marker layout into the list item's owning context - #11000
Conversation
Outside markers of list items that establish an independent formatting context were laid out from outside the boundary: the outer block context measured and placed them, create_used_values allocated marker entries recursively together with the list item's, and the list item's content block size was overwritten with the marker line-height after its own run had finished. Marker layout now runs in the epilogue of the list item's own block formatting context run; the outer context handles only markers of list items that participate in its flow directly. Outer float bands reach the run through a new input-only root sizing directive and the baseline comes from the run's derived baselines. The line-height floor is now part of automatic block size computation, so specified height and max-height win over it, matching other engines. Marker used values are created at one fixed point per configuration: layout_list_item_marker for in-flow outside markers, the inline-level iterator for markers in inline flow, the float machinery for floating markers, and the abspos engine for absolutely positioned ones. The empty-slot assert in create_used_values catches any second creator. Markers of absolutely positioned, floating, and flex-item list items were never laid out before and now work; the one rebaseline is such a marker changing from 0x0 to a placed box. New layout tests cover markers in flex-item, abspos, floating, flow-root, and overflow:hidden list items, marker baseline alignment in a flow-root list item, and specified heights winning over the floor; a ref test checks that overflow:hidden on a list item still clips its outside marker like other engines do.
📝 WalkthroughWalkthroughChangesList-item marker layout now creates used values from definite list-item dimensions, supports baseline placement, handles floats and absolute positioning, and passes float intrusion through child layout. Automatic block sizing excludes marker boxes from BFC contributions and floors automatic sizes by marker line height. New tests cover these cases. List-item marker layout
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: 1
🤖 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/block_formatting_context.rs`:
- Around line 2609-2621: Update the automatic block-size calculation around
automatic_block_size_for_block_level_element_disregarding_marker and
floor_list_item_automatic_block_size_by_marker_line_height so floating or
absolutely positioned markers return automatic_content_block_size unchanged.
Apply the same exclusion in the corresponding path near the other occurrence
(around the list-item sizing logic), while preserving the existing line-height
floor for in-flow markers.
🪄 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: 0799ac88-0575-420a-8ef9-f80f4dede28a
📒 Files selected for processing (20)
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/geometry.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_level_iterator.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsTests/LibWeb/Layout/expected/block-in-inline-in-floated-fieldset.txtTests/LibWeb/Layout/expected/list-item-specified-height-not-floored-by-marker.txtTests/LibWeb/Layout/expected/list-marker-in-abspos-list-item.txtTests/LibWeb/Layout/expected/list-marker-in-flex-item-list-item.txtTests/LibWeb/Layout/expected/list-marker-in-floating-list-item.txtTests/LibWeb/Layout/expected/list-marker-in-flow-root-and-overflow-hidden-list-item.txtTests/LibWeb/Layout/expected/list-marker-text-baseline-in-flow-root-list-item.txtTests/LibWeb/Layout/input/list-item-specified-height-not-floored-by-marker.htmlTests/LibWeb/Layout/input/list-marker-in-abspos-list-item.htmlTests/LibWeb/Layout/input/list-marker-in-flex-item-list-item.htmlTests/LibWeb/Layout/input/list-marker-in-floating-list-item.htmlTests/LibWeb/Layout/input/list-marker-in-flow-root-and-overflow-hidden-list-item.htmlTests/LibWeb/Layout/input/list-marker-text-baseline-in-flow-root-list-item.htmlTests/LibWeb/Ref/expected/overflow-hidden-list-item-clips-outside-marker-ref.htmlTests/LibWeb/Ref/input/overflow-hidden-list-item-clips-outside-marker.html
| ) -> CssPixels { | ||
| let automatic_content_block_size = self.automatic_block_size_for_block_level_element_disregarding_marker( | ||
| node, | ||
| available_space, | ||
| constraints, | ||
| automatic_content_block_size_of_completed_run, | ||
| ); | ||
| floor_list_item_automatic_block_size_by_marker_line_height( | ||
| self.state, | ||
| self.callbacks, | ||
| node, | ||
| automatic_content_block_size, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude out-of-flow markers from the automatic-size floor.
Lines 1411-1414 leave floating and absolutely positioned markers to their out-of-flow layout paths. This helper still raises the list item’s automatic block size from their line-height.
An auto-height list item can therefore grow for a large absolutely positioned marker. Return the original size when the marker is floating or absolutely positioned.
Proposed fix
let marker = facts.list_item_marker();
if marker.is_invalid() {
return automatic_content_block_size;
}
+ let marker_facts = state.node_facts(&callbacks, marker);
+ if marker_facts.is_floating() || marker_facts.is_absolutely_positioned() {
+ return automatic_content_block_size;
+ }
automatic_content_block_size.max(state.style_facts(&callbacks, marker).line_height())Also applies to: 2957-2979
🤖 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/Rust/src/layout/block_formatting_context.rs` around lines
2609 - 2621, Update the automatic block-size calculation around
automatic_block_size_for_block_level_element_disregarding_marker and
floor_list_item_automatic_block_size_by_marker_line_height so floating or
absolutely positioned markers return automatic_content_block_size unchanged.
Apply the same exclusion in the corresponding path near the other occurrence
(around the list-item sizing logic), while preserving the existing line-height
floor for in-flow markers.
10f4855
into
LadybirdBrowser:master
Outside markers of list items that establish an independent formatting context were laid out from outside the boundary: the outer block context measured and placed them, create_used_values allocated marker entries recursively together with the list item's, and the list item's content block size was overwritten with the marker line-height after its own run had finished.
Marker layout now runs in the epilogue of the list item's own block formatting context run; the outer context handles only markers of list items that participate in its flow directly. Outer float bands reach the run through a new input-only root sizing directive and the baseline comes from the run's derived baselines. The line-height floor is now part of automatic block size computation, so specified height and max-height win over it, matching other engines.
Marker used values are created at one fixed point per configuration: layout_list_item_marker for in-flow outside markers, the inline-level iterator for markers in inline flow, the float machinery for floating markers, and the abspos engine for absolutely positioned ones. The empty-slot assert in create_used_values catches any second creator.
Markers of absolutely positioned, floating, and flex-item list items were never laid out before and now work; the one rebaseline is such a marker changing from 0x0 to a placed box. New layout tests cover markers in flex-item, abspos, floating, flow-root, and overflow:hidden list items, marker baseline alignment in a flow-root list item, and specified heights winning over the floor; a ref test checks that overflow:hidden on a list item still clips its outside marker like other engines do.