Skip to content

feat(style): implement the CSS font shorthand - #135

Open
danperks wants to merge 1 commit into
lucid-softworks:mainfrom
danperks:feat/font-shorthand
Open

danperks wants to merge 1 commit into
lucid-softworks:mainfrom
danperks:feat/font-shorthand

Conversation

@danperks

Copy link
Copy Markdown

What & why

font was missing from the property dispatch entirely — the only "font" arm in
declaration.rs is the one that maps the legacy HTML <font> element's
attributes. So font: 10px/1 Ahem set nothing at all, and the element stayed at
the default 16px in the default family.

That matters more than a normal missing property, because font: <size>/<lh> Ahem
is the idiom WPT reaches for whenever a test needs predictable glyph metrics.
13.4% of files under css/ use the shorthand (6,574 of 49,014). Those tests
weren't failing because the feature under test was broken — they were being
measured with the wrong ruler, and a test whose reference uses the shorthand fails
even when the test itself never mentions it. On a 5,295-test sample across
css-text, css-flexbox, css-grid, css-backgrounds, css-sizing and
css-overflow:

pass fail total pass rate
test or its reference uses font: 170 1,406 1,576 10.8%
neither does 799 2,920 3,719 21.5%

1,406 of the 4,326 failures in that sample (32.5%) sat behind this.

What it does

Parses the full grammar:

[ <font-style> || <font-variant-css2> || <font-weight> || <font-stretch> ]?
<font-size> [ / <line-height> ]? <font-family>
  • The optional prefixes in any order, one per category, including numeric weights.
    font-variant and font-stretch aren't modelled by ComputedStyle, but they're
    still recognised — an unrecognised token has to invalidate the whole declaration,
    so silently not knowing them would reject valid values.
  • / <line-height> in all four spellings the tokenizer can hand over: 10px/1,
    10px/ 1, 10px /1, 10px / 1.
  • The <system-family-name> keywords (caption, menu, …). We can't ask the
    platform what those are, so they land on the UA default — but they still reset the
    longhands, which is the part that's observable.
  • inherit / unset (the font longhands all inherit, so those coincide) and
    initial.

Two behaviours worth calling out, both of which have tests:

  • Size and family are required. A value that yields neither is invalid and must
    leave the cascade alone, so nothing is written to the style until the whole value
    has parsed — font: 12px can't be allowed to land the size and drop the family.
  • It resets the longhands it covers. font-weight: bold; font: 20px serif ends
    up non-bold, and a line-height from an earlier declaration in the same rule
    doesn't survive. The line-height is resolved against this declaration's size,
    not the size the element had on the way in, so font: 10px/1 is 10px and not 16.

<absolute-size> keywords come along for the ride: the shorthand's <font-size>
accepts them and parse_font_size previously returned None for medium /
x-large / …, leaving the inherited size in place. They're now on the usual
medium = 16px scale.

Verified end-to-end as well as in unit tests — rendering font: 10px/1 monospace,
font: 10px monospace and font: italic bold 10px/2 monospace now produces line
boxes of 10px, 13px and 20px respectively, matching the equivalent longhands, where
before all three produced 21px (16px at the default line-height).

🤖 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

One note on the WPT delta

Expect this to show up as a mixed delta rather than a clean win, and the
regressions are worth reading carefully — several of them are tests that were only
passing because both sides were wrong in the same way. The clearest example is
css/css-grid/grid-items/grid-item-block-axis-content-contribution-00{1,2,3}:
test and reference both say font: 10px/1 Ahem, so with the shorthand ignored both
rendered at 16px and matched. With it honoured, the reference (explicit
height: 10px) is right and the test's auto-sized grid row comes out 13px, because
grid items ignore line-height — a separate, pre-existing bug that this change
merely stops masking. I'll send that one separately.

Made with Cursor

`font` was not in the property dispatch at all, so `font: 10px/1 Ahem` — the
idiom most of WPT uses to get predictable text metrics — left the element at the
default 16px in a default family. Every geometry assertion built on it was
therefore measured against the wrong font, which quietly distorts far more of the
suite than a missing feature normally would: 13.4% of `css/` tests use it.

Parses the full grammar, including the optional style/variant/weight/stretch
prefixes, `/ <line-height>` in each of its four spacings, and the
`<system-family-name>` keywords. Size and family are both required, so a value
yielding neither is dropped whole rather than half-applied, and a successful
parse resets the longhands it covers as a shorthand must.

`<absolute-size>` keywords (`medium`, `x-large`, …) come along with it: the
shorthand's `<font-size>` accepts them, and `parse_font_size` previously rejected
them and left the inherited size in place.

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