Resolve a rendered legend fit-content inline size before layout - #11047
Conversation
A fieldset's rendered legend with 'width: auto' uses its fit-content inline size. That size was applied as an override written to the legend's box after its layout returned, so the legend's children were laid out against a provisional stretch size. The fit-content rule now resolves inside the shared block-level inline-size resolution, before the legend's children are laid out, so they resolve against the used inline size. Covered by a new regression test.
📝 WalkthroughWalkthroughRendered fieldset legends with ChangesFieldset legend sizing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FieldsetLayout
participant RootInlineMetrics
participant RenderedLegend
FieldsetLayout->>RootInlineMetrics: Resolve root inline metrics
RootInlineMetrics->>RenderedLegend: Detect auto computed width
RootInlineMetrics->>RenderedLegend: Calculate fit-content inline size
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/block_formatting_context.rs`:
- Around line 519-537: Remove the early return in the block sizing flow and
store the fieldset rendered legend’s fit-content result from the
`style.width().is_auto()` branch as the common block-sizing input. Ensure that
value continues through the existing automatic margin resolution and
`min-width`/`max-width` calculations in the block sizing path, including the
logic around `SizingAxis::Inline` and the later sizing passes.
In
`@Tests/LibWeb/Text/input/fieldset-legend-independent-formatting-context-fit-content.html`:
- Line 7: Update the regression test’s child content and measurement logic:
replace the fixed-size .box setup with size-dependent or wrapping content, and
record each child’s rectangles in addition to the legend rectangles for every
formatting-context case covered by the test. Ensure the assertions observe child
layout at the legend’s available inline size rather than only the legend
geometry.
🪄 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: 3f4474c3-8b58-40ff-8fdd-f9a43137287e
📒 Files selected for processing (3)
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rsTests/LibWeb/Text/expected/fieldset-legend-independent-formatting-context-fit-content.txtTests/LibWeb/Text/input/fieldset-legend-independent-formatting-context-fit-content.html
| // https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements | ||
| // A rendered legend with a computed inline size of auto uses the | ||
| // fit-content size, resolved here so the legend's children are laid | ||
| // out against the used size rather than a provisional stretch size. | ||
| if style.width().is_auto() { | ||
| let container = self.containing_block(node); | ||
| if !container.is_invalid() | ||
| && self.facts(container).is_fieldset_box() | ||
| && self.facts(container).rendered_legend() == node | ||
| { | ||
| return Some(sizing.calculate_fit_content_size( | ||
| node, | ||
| SizingAxis::Inline, | ||
| remaining_available_space, | ||
| constraints, | ||
| )); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep fit-content sizing on the common block-sizing path.
The early return bypasses automatic margin resolution and the min-width/max-width passes at Lines 661-694. For example, max-width: 20px will not constrain an 80px legend, and margin-inline: auto will remain unresolved.
Store the fit-content value as the common path’s input, then apply the existing margin and min/max calculations.
🤖 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
519 - 537, Remove the early return in the block sizing flow and store the
fieldset rendered legend’s fit-content result from the `style.width().is_auto()`
branch as the common block-sizing input. Ensure that value continues through the
existing automatic margin resolution and `min-width`/`max-width` calculations in
the block sizing path, including the logic around `SizingAxis::Inline` and the
later sizing passes.
| body { margin: 0; } | ||
| fieldset { margin: 0; border: 2px solid black; padding: 10px; width: 300px; } | ||
| legend { padding: 0; } | ||
| .box { width: 40px; height: 16px; background: papayawhip; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the regression test observe child layout.
The fixed 40px children do not depend on the legend’s available inline size. The test also records only the legend rectangles. A regression that lays out children at the provisional fieldset width can therefore pass.
Add size-dependent or wrapping child content, and record the child rectangles for each formatting context.
Also applies to: 27-30
🤖 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/input/fieldset-legend-independent-formatting-context-fit-content.html`
at line 7, Update the regression test’s child content and measurement logic:
replace the fixed-size .box setup with size-dependent or wrapping content, and
record each child’s rectangles in addition to the legend rectangles for every
formatting-context case covered by the test. Ensure the assertions observe child
layout at the legend’s available inline size rather than only the legend
geometry.
abdb9c5
into
LadybirdBrowser:master
A fieldset's rendered legend with 'width: auto' uses its fit-content inline size. That size was applied as an override written to the legend's box after its layout returned, so the legend's children were laid out against a provisional stretch size. The fit-content rule now resolves inside the shared block-level inline-size resolution, before the legend's children are laid out, so they resolve against the used inline size. Covered by a new regression test.