From 219f8cf610642ba673bd9284678944cd9d141ada Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 11:01:24 +0200 Subject: [PATCH] fix(html): emit lengths the core parser can read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two parsers for the same syntax have drifted. Executed: `style="padding: 24px 48px; margin: 0 auto; grid-template-columns: repeat(3, 1fr)"` transpiles to `"padding":"24px 48px"`, `"margin":"0 auto"`, `"grid-template-columns":["repeat(3,","1fr)"]`. All three deserialize successfully — `Edges::Uniform(LengthPercentage::String)` and `GridTrack::Length(LengthPercentage::String)` are untagged string variants — and then core's `parse_length` (rustmotion-core/src/css/units.rs:125) only understands a single number+unit token, so `parse_length_or_warn` (units.rs:262) falls back to `Px(0.0)`: zero padding, zero margin, two zero-width grid tracks. The only signal is an `eprintln!` warning; `validate` exits 0 and the render is wrong. Per-side padding is entirely unexpressible from HTML (`padding-top:32` is rejected by `CssStyle`'s `deny_unknown_fields`, `padding:32px 48px` silently becomes 0), and the skill rule html-css-mental-model.md:97 only documents the JSON object form, which inline `style=""` cannot produce. Refs #220 --- crates/rustmotion-html/src/element.rs | 9 +- crates/rustmotion-html/src/style.rs | 217 +++++++++++++++++++-- crates/rustmotion-html/tests/audit_ws_f.rs | 92 +++++++++ 3 files changed, 298 insertions(+), 20 deletions(-) diff --git a/crates/rustmotion-html/src/element.rs b/crates/rustmotion-html/src/element.rs index b370f658..337c8b2e 100644 --- a/crates/rustmotion-html/src/element.rs +++ b/crates/rustmotion-html/src/element.rs @@ -58,11 +58,10 @@ fn collect_text(handle: &Handle, out: &mut String) { /// [`crate::style::parse_anim_attr`]) lands in `style.animation` — inline CSS /// cannot express animation arrays, so `anim` is the only writer of that key. fn style_object(attrs: &[(String, String)]) -> Result, HtmlError> { - let mut map = attrs - .iter() - .find(|(k, _)| k == "style") - .map(|(_, raw)| parse_inline_style(raw)) - .unwrap_or_default(); + let mut map = match attrs.iter().find(|(k, _)| k == "style") { + Some((_, raw)) => parse_inline_style(raw)?, + None => Map::new(), + }; if let Some((_, anim)) = attrs.iter().find(|(k, _)| k == "anim") { map.insert("animation".into(), parse_anim_attr(anim)?); } diff --git a/crates/rustmotion-html/src/style.rs b/crates/rustmotion-html/src/style.rs index d27a71e4..e80a239c 100644 --- a/crates/rustmotion-html/src/style.rs +++ b/crates/rustmotion-html/src/style.rs @@ -28,10 +28,16 @@ pub fn coerce_value(raw: &str) -> Value { Value::from(t.to_string()) } -/// Parse an inline `style="a:b; c:d"` declaration list into a JSON style object. -/// `grid-template-columns`/`-rows` are split into string arrays; all other -/// properties pass through their kebab-case name with a coerced value. -pub fn parse_inline_style(decls: &str) -> Map { +/// Parse an inline `style="a:b; c:d"` declaration list into a JSON style +/// object. `padding`/`margin`/`border-radius` accept the CSS 1/2/3/4-value +/// box shorthand, expanded into the `{top,right,bottom,left}` / +/// `{top-left,top-right,bottom-right,bottom-left}` object the core CSS +/// engine's `Edges`/`BorderRadius` types deserialize. `grid-template-columns`/ +/// `-rows` accept a track list, `repeat()`/`minmax()` included. Any other +/// property whose value is more than one top-level (paren-aware) token is +/// refused rather than passed through as an opaque string the core length +/// parser cannot read (see [`HtmlError::UnsupportedStyleShorthand`]). +pub fn parse_inline_style(decls: &str) -> Result, HtmlError> { let mut map = Map::new(); for decl in decls.split(';') { let decl = decl.trim(); @@ -43,17 +49,198 @@ pub fn parse_inline_style(decls: &str) -> Map { }; let prop = prop.trim().to_string(); let value = value.trim(); - if prop == "grid-template-columns" || prop == "grid-template-rows" { - let arr: Vec = value - .split_whitespace() - .map(|t| Value::from(t.to_string())) - .collect(); - map.insert(prop, Value::Array(arr)); - } else { - map.insert(prop, coerce_value(value)); + match prop.as_str() { + "grid-template-columns" | "grid-template-rows" => { + map.insert( + prop.clone(), + Value::Array(parse_grid_template(&prop, value)?), + ); + } + "padding" | "margin" => { + let tokens = split_top_level_tokens(value); + match tokens.len() { + 1 => { + map.insert(prop, coerce_value(value)); + } + 2..=4 => { + map.insert(prop, expand_box_edges(&tokens)); + } + _ => { + return Err(HtmlError::UnsupportedStyleShorthand { + prop, + value: value.to_string(), + }) + } + } + } + "border-radius" => { + let tokens = split_top_level_tokens(value); + match tokens.len() { + 1 => { + map.insert(prop, coerce_value(value)); + } + 2..=4 => { + map.insert(prop, expand_border_radius_corners(&tokens)); + } + _ => { + return Err(HtmlError::UnsupportedStyleShorthand { + prop, + value: value.to_string(), + }) + } + } + } + _ => { + if split_top_level_tokens(value).len() > 1 { + return Err(HtmlError::UnsupportedStyleShorthand { + prop, + value: value.to_string(), + }); + } + map.insert(prop, coerce_value(value)); + } + } + } + Ok(map) +} + +/// Split a CSS value on top-level whitespace: whitespace inside a `(...)` +/// span (e.g. the argument list of `rgba(0, 0, 0, 0.5)` or `repeat(3, 1fr)`) +/// does not count as a separator, so a single functional-notation value +/// stays one token while a genuine multi-value shorthand (`24px 48px`) +/// splits into its parts. +fn split_top_level_tokens(s: &str) -> Vec<&str> { + let mut tokens = Vec::new(); + let mut depth = 0i32; + let mut token_start: Option = None; + for (i, c) in s.char_indices() { + match c { + '(' => depth += 1, + ')' => depth = depth.saturating_sub(1), + _ => {} + } + if c.is_whitespace() && depth == 0 { + if let Some(start) = token_start.take() { + tokens.push(&s[start..i]); + } + } else if token_start.is_none() { + token_start = Some(i); + } + } + if let Some(start) = token_start { + tokens.push(&s[start..]); + } + tokens +} + +/// Expand a 2/3/4-value `padding`/`margin` shorthand into the +/// `{top,right,bottom,left}` object `Edges::Sides` deserializes, following +/// the standard CSS clockwise-from-top expansion rule. +fn expand_box_edges(tokens: &[&str]) -> Value { + let (top, right, bottom, left) = match tokens { + [a, b] => (*a, *b, *a, *b), + [a, b, c] => (*a, *b, *c, *b), + [a, b, c, d] => (*a, *b, *c, *d), + _ => unreachable!("caller only passes 2..=4 tokens"), + }; + let mut edges = Map::new(); + edges.insert("top".into(), coerce_value(top)); + edges.insert("right".into(), coerce_value(right)); + edges.insert("bottom".into(), coerce_value(bottom)); + edges.insert("left".into(), coerce_value(left)); + Value::Object(edges) +} + +/// Expand a 2/3/4-value `border-radius` shorthand into the +/// `{top-left,top-right,bottom-right,bottom-left}` object +/// `BorderRadius::Corners` deserializes, following the standard CSS +/// clockwise-from-top-left expansion rule (a different starting corner than +/// [`expand_box_edges`], per the CSS box-shorthand spec). +fn expand_border_radius_corners(tokens: &[&str]) -> Value { + let (top_left, top_right, bottom_right, bottom_left) = match tokens { + [a, b] => (*a, *b, *a, *b), + [a, b, c] => (*a, *b, *c, *b), + [a, b, c, d] => (*a, *b, *c, *d), + _ => unreachable!("caller only passes 2..=4 tokens"), + }; + let mut corners = Map::new(); + corners.insert("top-left".into(), coerce_value(top_left)); + corners.insert("top-right".into(), coerce_value(top_right)); + corners.insert("bottom-right".into(), coerce_value(bottom_right)); + corners.insert("bottom-left".into(), coerce_value(bottom_left)); + Value::Object(corners) +} + +/// Parse a `grid-template-columns`/`-rows` track list into the flat +/// `Vec` JSON the core CSS engine expects: `repeat(n, track)` +/// expands into `n` copies of `track`, `minmax(min, max)` becomes +/// `{"min":..,"max":..}`, and every other token passes through +/// [`coerce_value`] unchanged (a bare number for `fr`, a keyword string, or +/// an explicit length). +fn parse_grid_template(prop: &str, value: &str) -> Result, HtmlError> { + let mut out = Vec::new(); + for token in split_top_level_tokens(value) { + push_grid_track(prop, token, &mut out)?; + } + Ok(out) +} + +fn push_grid_track(prop: &str, token: &str, out: &mut Vec) -> Result<(), HtmlError> { + if let Some(inner) = token + .strip_prefix("repeat(") + .and_then(|s| s.strip_suffix(')')) + { + let (count_str, pattern) = + inner + .split_once(',') + .ok_or_else(|| HtmlError::UnsupportedStyleShorthand { + prop: prop.to_string(), + value: token.to_string(), + })?; + let count: usize = + count_str + .trim() + .parse() + .map_err(|_| HtmlError::UnsupportedStyleShorthand { + prop: prop.to_string(), + value: token.to_string(), + })?; + let pattern_tokens = split_top_level_tokens(pattern.trim()); + if pattern_tokens.is_empty() { + return Err(HtmlError::UnsupportedStyleShorthand { + prop: prop.to_string(), + value: token.to_string(), + }); } + for _ in 0..count { + for t in &pattern_tokens { + out.push(parse_single_grid_track(prop, t)?); + } + } + return Ok(()); + } + out.push(parse_single_grid_track(prop, token)?); + Ok(()) +} + +fn parse_single_grid_track(prop: &str, token: &str) -> Result { + if let Some(inner) = token + .strip_prefix("minmax(") + .and_then(|s| s.strip_suffix(')')) + { + let (min_s, max_s) = + inner + .split_once(',') + .ok_or_else(|| HtmlError::UnsupportedStyleShorthand { + prop: prop.to_string(), + value: token.to_string(), + })?; + let mut minmax = Map::new(); + minmax.insert("min".into(), coerce_value(min_s.trim())); + minmax.insert("max".into(), coerce_value(max_s.trim())); + return Ok(Value::Object(minmax)); } - map + Ok(coerce_value(token)) } /// Parse an `anim` attribute into the `style.animation` JSON array. @@ -187,7 +374,7 @@ mod tests { #[test] fn parses_declarations_into_style_object() { - let m = parse_inline_style("font-size:96px; color:#fff; text-align:center"); + let m = parse_inline_style("font-size:96px; color:#fff; text-align:center").unwrap(); assert_eq!(m.get("font-size"), Some(&json!(96))); assert_eq!(m.get("color"), Some(&json!("#fff"))); assert_eq!(m.get("text-align"), Some(&json!("center"))); @@ -195,7 +382,7 @@ mod tests { #[test] fn grid_template_becomes_string_array() { - let m = parse_inline_style("grid-template-columns: 1fr 1fr"); + let m = parse_inline_style("grid-template-columns: 1fr 1fr").unwrap(); assert_eq!(m.get("grid-template-columns"), Some(&json!(["1fr", "1fr"]))); } diff --git a/crates/rustmotion-html/tests/audit_ws_f.rs b/crates/rustmotion-html/tests/audit_ws_f.rs index 2230e257..60e4a8a1 100644 --- a/crates/rustmotion-html/tests/audit_ws_f.rs +++ b/crates/rustmotion-html/tests/audit_ws_f.rs @@ -95,3 +95,95 @@ fn inert_attributes_stay_inert_on_container_and_text() { let html = r##"

Hi

"##; html_to_scenario_value(html).expect("class/id/data-* must remain inert, not flagged"); } + +// --------------------------------------------------------------------------- +// CSS shorthand values transpile to strings the core length parser +// cannot read, silently resolving to 0px. +// --------------------------------------------------------------------------- + +#[test] +fn padding_two_value_shorthand_expands_to_edges_object() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("2-value padding must transpile"); + let padding = &v["scenes"][0]["children"][0]["style"]["padding"]; + assert_eq!(padding["top"], json!(24)); + assert_eq!(padding["bottom"], json!(24)); + assert_eq!(padding["right"], json!(48)); + assert_eq!(padding["left"], json!(48)); +} + +#[test] +fn margin_four_value_shorthand_expands_to_edges_object() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("4-value margin must transpile"); + let margin = &v["scenes"][0]["children"][0]["style"]["margin"]; + assert_eq!(margin["top"], json!(4)); + assert_eq!(margin["right"], json!(8)); + assert_eq!(margin["bottom"], json!(12)); + assert_eq!(margin["left"], json!(16)); +} + +#[test] +fn border_radius_four_value_shorthand_expands_to_corners_object() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("4-value border-radius must transpile"); + let radius = &v["scenes"][0]["children"][0]["style"]["border-radius"]; + assert_eq!(radius["top-left"], json!(2)); + assert_eq!(radius["top-right"], json!(4)); + assert_eq!(radius["bottom-right"], json!(6)); + assert_eq!(radius["bottom-left"], json!(8)); +} + +#[test] +fn grid_template_columns_repeat_expands_to_flat_track_list() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("repeat() must transpile"); + let tracks = v["scenes"][0]["children"][0]["style"]["grid-template-columns"] + .as_array() + .expect("array of tracks"); + assert_eq!(tracks, &vec![json!("1fr"), json!("1fr"), json!("1fr")]); +} + +#[test] +fn grid_template_columns_minmax_expands_to_min_max_object() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("minmax() must transpile"); + let tracks = v["scenes"][0]["children"][0]["style"]["grid-template-columns"] + .as_array() + .expect("array of tracks"); + assert_eq!(tracks.len(), 2); + assert_eq!(tracks[0]["min"], json!(100)); + assert_eq!(tracks[0]["max"], json!("1fr")); + assert_eq!(tracks[1], json!("auto")); +} + +#[test] +fn unhandled_multi_token_style_value_is_refused_not_an_opaque_string() { + let html = r##"
"##; + let err = html_to_scenario_value(html) + .expect_err("an unsupported multi-token style value must be refused, not silently zeroed"); + match err { + HtmlError::UnsupportedStyleShorthand { prop, value } => { + assert_eq!(prop, "gap"); + assert_eq!(value, "8px 16px"); + } + other => panic!("expected UnsupportedStyleShorthand, got: {other:?}"), + } +} + +#[test] +fn single_token_padding_still_transpiles_to_a_plain_value() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("uniform padding must still transpile"); + assert_eq!(v["scenes"][0]["children"][0]["style"]["padding"], json!(32)); +} + +#[test] +fn rgba_color_functional_notation_is_not_treated_as_multi_token() { + let html = r##"
"##; + let v = html_to_scenario_value(html).expect("rgba(...) must not be flagged as multi-token"); + assert_eq!( + v["scenes"][0]["children"][0]["style"]["background"], + json!("rgba(0, 0, 0, 0.5)") + ); +}