Skip to content

fix(components): resolve default sizes against the node font-size - #263

Merged
LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/default-size-font-context
Sep 22, 2026
Merged

LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/default-size-font-context

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Severity Medium, category correctness. Location: crates/rustmotion-components/src/box_builder.rs:1716

Impact

font_size_px_or goes through Length::px() → px_or_warn, which returns 0.0 for any relative unit; style.rs:1601 asserts this explicitly (assert_eq!(s.font_size_px_or(48.0), 0.0) for "15.6vw"). Five live call sites remain in apply_intrinsic_overrides — List (1498), Callout (1643), Tooltip (1663), PillNav (1688), Marquee (1716) — while every corresponding painter was migrated to the context-aware resolver in the "lot B, wave S" pass (marquee.rs:82 self.style.font_size_px_ctx(&crate::intrinsic::font_size_ctx(...), self.font_size), list.rs:88 the same). So a marquee with "font-size": "2rem" and no explicit style.height gets apply_default_size(css, 800.0, 0.0) → the (false,false) branch writes css.height = Px(0.0) → the node is skipped at paint time and the marquee is invisible; list (1498-1503) collapses to height 0 the same way; callout/tooltip/pill_nav get boxes measured at font-size 0 while the painter draws at the real size, so the ink overflows its own box. intrinsic.rs:27-46 states this wave's goal was to replace font_size_px_or at every site; box_builder was missed, and it is also the one file that does not use the module's own measure_time_font_size_ctx helper (intrinsic.rs:87).

Fix

Replace the five font_size_px_or(...) calls in apply_intrinsic_overrides with font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), ...), the same helper every *Intrinsic in intrinsic.rs already uses. Add a regression test asserting a marquee/list with "font-size": "2rem" and no explicit height lays out with positive height.

Evidence the audit read

let font_size = m.style.font_size_px_or(m.font_size);
            apply_default_size(css, 800.0, font_size * 2.0);

Based directly on the chantier branch.

Part of the September 2026 audit remediation chantier. Refs #220 (RM-06).

@LeadcodeDev LeadcodeDev added the bug Something isn't working label Sep 21, 2026
@LeadcodeDev LeadcodeDev self-assigned this Sep 21, 2026
@LeadcodeDev
LeadcodeDev force-pushed the fix/default-size-font-context branch 2 times, most recently from f08bbe7 to 2351a9f Compare September 22, 2026 08:36
font_size_px_or goes through Length::px() → px_or_warn, which returns 0.0
for any relative unit; style.rs:1601 asserts this explicitly
(assert_eq!(s.font_size_px_or(48.0), 0.0) for "15.6vw"). Five live call
sites remain in apply_intrinsic_overrides — List (1498), Callout (1643),
Tooltip (1663), PillNav (1688), Marquee (1716) — while every corresponding
painter was migrated to the context-aware resolver in the "lot B, wave S"
pass (marquee.rs:82
self.style.font_size_px_ctx(&crate::intrinsic::font_size_ctx(...),
self.font_size), list.rs:88 the same). So a marquee with "font-size": "2rem"
and no explicit style.height gets apply_default_size(css, 800.0, 0.0) → the
(false,false) branch writes css.height = Px(0.0) → the node is skipped at
paint time and the marquee is invisible; list (1498-1503) collapses to
height 0 the same way; callout/tooltip/pill_nav get boxes measured at font-
size 0 while the painter draws at the real size, so the ink overflows its
own box. intrinsic.rs:27-46 states this wave's goal was to replace
font_size_px_or at every site; box_builder was missed, and it is also the
one file that does not use the module's own measure_time_font_size_ctx
helper (intrinsic.rs:87).

Fix: Replace the five font_size_px_or(...) calls in
apply_intrinsic_overrides with
font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), ...),
the same helper every *Intrinsic in intrinsic.rs already uses. Add a
regression test asserting a marquee/list with "font-size": "2rem" and no
explicit height lays out with positive height.

Refs #220
@LeadcodeDev
LeadcodeDev force-pushed the fix/default-size-font-context branch from 2351a9f to 067ad3a Compare September 22, 2026 08:46
@LeadcodeDev
LeadcodeDev merged commit 4099977 into chantier/audit-2026-09 Sep 22, 2026
0 of 3 checks passed
@LeadcodeDev
LeadcodeDev deleted the fix/default-size-font-context branch September 22, 2026 08:54
LeadcodeDev added a commit that referenced this pull request Sep 22, 2026
font_size_px_or goes through Length::px() → px_or_warn, which returns 0.0
for any relative unit; style.rs:1601 asserts this explicitly
(assert_eq!(s.font_size_px_or(48.0), 0.0) for "15.6vw"). Five live call
sites remain in apply_intrinsic_overrides — List (1498), Callout (1643),
Tooltip (1663), PillNav (1688), Marquee (1716) — while every corresponding
painter was migrated to the context-aware resolver in the "lot B, wave S"
pass (marquee.rs:82
self.style.font_size_px_ctx(&crate::intrinsic::font_size_ctx(...),
self.font_size), list.rs:88 the same). So a marquee with "font-size": "2rem"
and no explicit style.height gets apply_default_size(css, 800.0, 0.0) → the
(false,false) branch writes css.height = Px(0.0) → the node is skipped at
paint time and the marquee is invisible; list (1498-1503) collapses to
height 0 the same way; callout/tooltip/pill_nav get boxes measured at font-
size 0 while the painter draws at the real size, so the ink overflows its
own box. intrinsic.rs:27-46 states this wave's goal was to replace
font_size_px_or at every site; box_builder was missed, and it is also the
one file that does not use the module's own measure_time_font_size_ctx
helper (intrinsic.rs:87).

Fix: Replace the five font_size_px_or(...) calls in
apply_intrinsic_overrides with
font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), ...),
the same helper every *Intrinsic in intrinsic.rs already uses. Add a
regression test asserting a marquee/list with "font-size": "2rem" and no
explicit height lays out with positive height.

Refs #220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant