Skip to content

fix(layout): honour line-height in the intrinsic cross size - #136

Open
danperks wants to merge 1 commit into
lucid-softworks:mainfrom
danperks:fix/intrinsic-cross-line-height
Open

danperks wants to merge 1 commit into
lucid-softworks:mainfrom
danperks:fix/intrinsic-cross-line-height

Conversation

@danperks

Copy link
Copy Markdown

What & why

intrinsic_cross_height in crates/layout/src/flex.rs estimated an item's
single-line height straight from the font metric:

let line_h = if has_any_text(boxx) {
    measurer.line_height(if fs > 0.0 { fs } else { 16.0 }, boxx.style.font_family.as_deref())
} else { 0.0 };

A specified line-height never entered into it. Everywhere else the two are combined
the specified value wins and the metric is the fallback — inline.rs, build.rs and
intrinsic.rs all use style.line_height.unwrap_or_else(|| measurer.line_height(…)).

That would be a harmless over-guess if the estimate were only a floor, but
resolve_row_heights takes the greater of this estimate and the item's laid-out
height. So whenever line-height is shorter than the font's natural metric, the
estimate wins and the auto row is laid out taller than the content it contains: one
line of 10px text with line-height: 10px gave a 13px row.

Reproduced against a grid item and its longhand-styled equivalent — the row measured
13px where a plain block with the same styles correctly measured 10px.

This is also what makes css/css-grid/grid-items/grid-item-block-axis-content-contribution-00{1,2,3}
fail: those compare an auto-sized grid row against a reference with an explicit
height: 10px, and with this they match exactly (0 differing pixels). They currently
"pass" only because font: 10px/1 Ahem is ignored on both sides (see #135), so both
render at 16px and the row height never gets checked. With #135 landed and this one
not, they fail; with both, they pass for the right reason.

The fix is to prefer the specified value, as the other three call sites do. The test
covers both directions, so it can't regress to a different hard-coded answer:
line-height: 10px gives a 10px row, and line-height: normal still gives the
font metric's 13px.

🤖 How this was built

  • Authored primarily with an LLM (which: Claude Opus 4.5 via Cursor)

Checklist

  • PR title is a Conventional Commit
  • cargo test --workspace passes
  • cargo fmt --all + cargo clippy clean
  • WPT report (auto-commented below) shows no conformance regressions

Made with Cursor

`intrinsic_cross_height` estimated one line of text from the font metric alone,
ignoring a specified `line-height`. Grid takes the greater of that estimate and the
item's laid-out height, so the estimate isn't a harmless over-guess — when
`line-height` is shorter than the font's natural metric the estimate wins and the
auto row is laid out taller than its content. An item with one line of 10px text and
`line-height: 10px` produced a 13px row.

Every other place that combines the two prefers the specified value and falls back
to the metric; this one now does the same.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant