Skip to content

Commit fa0eb84

Browse files
LeadcodeDevLeadcodeDev
committed
fix(html): turn the dialect's silent losses into named errors
The HTML dialect degraded four classes of input without a word, and `rustmotion validate` answered "Valid scenario" for all of them. An author who writes a construct the transpiler cannot honour has to be told. `<style>` content was transpiled into a `text` component and painted into the video. It is now refused: `<style>` has a real, expected visual effect that the dialect cannot deliver (there is no cascade engine), so dropping it silently defeats a genuine intent. `<script>`, `<title>`, `<noscript>`, `<template>` and `<head>` are skipped instead — no browser paints them, so ignoring them defeats nothing and merely stops their text leaking onto the canvas. A `<scene>` nested inside any container disappeared from the scenario. It is now refused, naming the offending parent, and the search is recursive: that also catches the case an unclosed `<p>` creates, where HTML5 error recovery hoists the `<h1>` out of the scene and leaves an empty one behind. Recursing silently would have hidden exactly that corruption. `<img>`, `<video>` and `<svg>` became empty `div`s. They are now refused with the equivalent `rm-*` element named in the message. No `bool` schema field was reachable: `coerce_value` left `"false"` a string, so `auto_scroll`, `diff`, `loop`, `show_grid` and friends could not be expressed at all. It now matches `coerce_dsl_value`, and a bare HTML boolean attribute resolves to `true`.
1 parent 1a351ff commit fa0eb84

4 files changed

Lines changed: 450 additions & 5 deletions

File tree

‎crates/rustmotion-html/src/element.rs‎

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ use crate::{element_attrs, tag_name, HtmlError};
77
enum TagKind {
88
Container,
99
Text,
10+
/// Tags that never visually render in real HTML either (`<script>`,
11+
/// `<title>`, `<noscript>`, `<template>`, `<head>`) — skipped to match
12+
/// that expectation, rather than painted as a stray `text` component.
13+
/// `<style>` is deliberately NOT in this bucket: see `element_to_value`.
14+
Ignored,
15+
/// A native HTML tag with no representation beyond an empty `div`: its
16+
/// real payload (`src`, nested shape markup, …) would be silently
17+
/// dropped by the generic `Container` fallback. Refused instead, naming
18+
/// the dialect's `rm-*` custom-element equivalent.
19+
UnsupportedNative(&'static str),
1020
Custom(String),
1121
}
1222

@@ -16,6 +26,10 @@ fn tag_kind(tag: &str) -> TagKind {
1626
"p" | "span" | "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "strong" | "em" | "label" => {
1727
TagKind::Text
1828
}
29+
"script" | "title" | "noscript" | "template" | "head" => TagKind::Ignored,
30+
"img" => TagKind::UnsupportedNative("rm-image"),
31+
"video" => TagKind::UnsupportedNative("rm-video"),
32+
"svg" => TagKind::UnsupportedNative("rm-svg"),
1933
t if t.starts_with("rm-") => TagKind::Custom(t["rm-".len()..].to_string()),
2034
_ => TagKind::Container,
2135
}
@@ -62,8 +76,19 @@ pub(crate) fn element_to_value(handle: &Handle) -> Result<Option<Value>, HtmlErr
6276
let Some(tag) = tag_name(handle) else {
6377
return Ok(None);
6478
};
79+
// `<style>` has real, expected visual effect in HTML (unlike the tags in
80+
// `TagKind::Ignored`), so silently dropping it would defeat the author's
81+
// intent without a trace — refused instead. See `HtmlError::StyleElementUnsupported`.
82+
if tag == "style" {
83+
return Err(HtmlError::StyleElementUnsupported);
84+
}
6585
let attrs = element_attrs(handle);
6686
match tag_kind(&tag) {
87+
TagKind::Ignored => Ok(None),
88+
TagKind::UnsupportedNative(suggestion) => Err(HtmlError::UnsupportedNativeElement {
89+
tag,
90+
suggestion: suggestion.to_string(),
91+
}),
6792
TagKind::Text => {
6893
let mut obj = Map::new();
6994
obj.insert("type".into(), Value::from("text"));
@@ -89,10 +114,24 @@ pub(crate) fn element_to_value(handle: &Handle) -> Result<Option<Value>, HtmlErr
89114
let mut obj = Map::new();
90115
obj.insert("type".into(), Value::from(type_name));
91116
for (k, v) in &attrs {
92-
if k == "style" || k == "class" || k == "anim" || v.is_empty() {
117+
if k == "style" || k == "class" || k == "anim" {
93118
continue;
94119
}
95-
obj.insert(k.clone(), coerce_value(v));
120+
// A bare HTML boolean attribute (`<rm-codeblock diff>`) is
121+
// indistinguishable, at the DOM level, from an explicit empty
122+
// value (`diff=""`) — html5ever normalizes both to the same
123+
// empty attribute value. Per HTML's own boolean-attribute
124+
// convention (`<video controls>`, `<input disabled>`), treat
125+
// an empty value as `true` rather than silently dropping the
126+
// attribute: for a bool schema field this is exactly the
127+
// author's intent; for any other field type, `validate`
128+
// reports a named type-mismatch instead of a silent no-op.
129+
let value = if v.is_empty() {
130+
Value::Bool(true)
131+
} else {
132+
coerce_value(v)
133+
};
134+
obj.insert(k.clone(), value);
96135
}
97136
if let Some(style) = style_object(&attrs)? {
98137
obj.insert("style".into(), style);
@@ -327,4 +366,108 @@ mod tests {
327366
assert_eq!(v["style"]["animation"][0]["name"], json!("fade_in"));
328367
assert_eq!(v["from"], json!(0));
329368
}
369+
370+
// --- <style>/ignored elements (constat 1) ---
371+
372+
#[test]
373+
fn style_element_is_refused() {
374+
let e = map_first_err(r#"<style>h1 { color: #0f0 }</style>"#);
375+
assert!(
376+
matches!(e, crate::HtmlError::StyleElementUnsupported),
377+
"expected StyleElementUnsupported, got: {e:?}"
378+
);
379+
}
380+
381+
#[test]
382+
fn script_element_is_skipped_not_painted() {
383+
let v = map_first(r#"<div><script>alert(1)</script><p>real</p></div>"#);
384+
let children = v["children"].as_array().expect("children array");
385+
assert_eq!(
386+
children.len(),
387+
1,
388+
"script content must not become a component: {v}"
389+
);
390+
assert_eq!(children[0]["content"], json!("real"));
391+
}
392+
393+
#[test]
394+
fn title_and_noscript_and_template_elements_are_skipped_not_painted() {
395+
let v = map_first(
396+
r#"<div><title>tt</title><noscript>ns</noscript><template>tpl</template><p>real</p></div>"#,
397+
);
398+
let children = v["children"].as_array().expect("children array");
399+
assert_eq!(
400+
children.len(),
401+
1,
402+
"title/noscript/template content must not become a component: {v}"
403+
);
404+
assert_eq!(children[0]["content"], json!("real"));
405+
}
406+
407+
#[test]
408+
fn tag_kind_head_is_ignored() {
409+
// <head> content never survives as a distinct DOM node when authored
410+
// inline (html5ever drops the wrapper per HTML5 "in body" parsing
411+
// rules and lets its text bleed into the parent), so this can only be
412+
// exercised at the `tag_kind` unit level, not through the full
413+
// element_to_value/html_to_scenario_value pipeline.
414+
assert!(matches!(tag_kind("head"), TagKind::Ignored));
415+
}
416+
417+
// --- unsupported native elements (constat 3) ---
418+
419+
#[test]
420+
fn img_element_is_refused_with_rm_image_suggestion() {
421+
let e = map_first_err(r#"<img src="hero.png" width="400" height="300">"#);
422+
match e {
423+
crate::HtmlError::UnsupportedNativeElement { tag, suggestion } => {
424+
assert_eq!(tag, "img");
425+
assert_eq!(suggestion, "rm-image");
426+
}
427+
other => panic!("expected UnsupportedNativeElement, got: {other:?}"),
428+
}
429+
}
430+
431+
#[test]
432+
fn video_element_is_refused_with_rm_video_suggestion() {
433+
let e = map_first_err(r#"<video src="clip.mp4"></video>"#);
434+
match e {
435+
crate::HtmlError::UnsupportedNativeElement { tag, suggestion } => {
436+
assert_eq!(tag, "video");
437+
assert_eq!(suggestion, "rm-video");
438+
}
439+
other => panic!("expected UnsupportedNativeElement, got: {other:?}"),
440+
}
441+
}
442+
443+
#[test]
444+
fn svg_element_is_refused_with_rm_svg_suggestion() {
445+
let e = map_first_err(r#"<svg viewBox="0 0 10 10"><circle r="4"></circle></svg>"#);
446+
match e {
447+
crate::HtmlError::UnsupportedNativeElement { tag, suggestion } => {
448+
assert_eq!(tag, "svg");
449+
assert_eq!(suggestion, "rm-svg");
450+
}
451+
other => panic!("expected UnsupportedNativeElement, got: {other:?}"),
452+
}
453+
}
454+
455+
// --- boolean attributes on custom elements (constat 4) ---
456+
457+
#[test]
458+
fn custom_element_bool_attribute_true_and_false() {
459+
let v = map_first(r#"<rm-codeblock auto_scroll="false" diff="true"></rm-codeblock>"#);
460+
assert_eq!(v["auto_scroll"], json!(false));
461+
assert_eq!(v["diff"], json!(true));
462+
}
463+
464+
#[test]
465+
fn custom_element_bare_attribute_becomes_true() {
466+
let v = map_first(r#"<rm-codeblock diff></rm-codeblock>"#);
467+
assert_eq!(
468+
v["diff"],
469+
json!(true),
470+
"bare boolean attribute must become true, not be dropped: {v}"
471+
);
472+
}
330473
}

‎crates/rustmotion-html/src/lib.rs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,34 @@ pub enum HtmlError {
6969
/// Emitted when `<font>` sets both `path`/`src` and `source` — they are mutually exclusive.
7070
#[error("<font family=\"{family}\">: 'path'/'src' and 'source' are mutually exclusive")]
7171
FontPathAndSourceConflict { family: String },
72+
/// Emitted for `<style>`. Unlike `<script>`/`<title>`/`<noscript>`/`<template>`
73+
/// (silently skipped — they never visually render in real HTML either, so
74+
/// skipping them matches an author's own expectation), `<style>` DOES have
75+
/// real, expected visual effect in HTML. The dialect has no CSS
76+
/// selector/cascade engine (only inline `style="..."` attributes), so a
77+
/// `<style>` block's rules would never take effect — refused instead of
78+
/// silently discarded, so the author's intent isn't dropped without a trace.
79+
#[error(
80+
"<style> blocks are not supported by the HTML dialect (no CSS selector/cascade engine) — move these declarations onto the target elements' style=\"...\" attribute"
81+
)]
82+
StyleElementUnsupported,
83+
/// Emitted for a native HTML tag whose real payload (`src`, nested shape
84+
/// markup, …) has no representation via the generic `Container`/`div`
85+
/// fallback — that fallback would silently render an empty box. The
86+
/// dialect's `rm-*` custom-element mechanism is the way to express these.
87+
#[error(
88+
"<{tag}> is not supported by the HTML dialect and would render as an empty container — use <{suggestion} ...> instead"
89+
)]
90+
UnsupportedNativeElement { tag: String, suggestion: String },
91+
/// Emitted when a `<scene>` is found nested inside an element other than
92+
/// `<rustmotion>` itself (or `<font>`, which the transpiler recurses
93+
/// through to work around html5ever's formatting-element reconstruction).
94+
/// `collect_scenes_and_fonts` only walks direct children, so a nested
95+
/// `<scene>` would otherwise vanish from the scenario without a trace.
96+
#[error(
97+
"<scene> found nested inside <{parent}> — <scene> elements must be direct children of <rustmotion> (only <font> is recursed into)"
98+
)]
99+
NestedScene { parent: String },
72100
}
73101

74102
/// Transpile an HTML-dialect document into the scenario `serde_json::Value` that
@@ -169,6 +197,15 @@ fn font_to_value(handle: &Handle) -> Result<Value, HtmlError> {
169197
/// element and nests subsequent siblings inside it, we recurse into `<font>`
170198
/// children so that `<scene>` elements placed after `<font>` declarations are
171199
/// still found at any depth.
200+
///
201+
/// Any other child is scanned (at any depth) for a nested `<scene>` — a
202+
/// wrapper element (typo'd unclosed tag, deliberate `<div>` grouping, or
203+
/// html5ever's own formatting-element error recovery on tags like `<b>`)
204+
/// would otherwise make `<scene>` elements vanish from the scenario with no
205+
/// trace, since this function only descends into direct children. A `<style>`
206+
/// found at this level is refused for the same reason `element_to_value`
207+
/// refuses it inside a scene: it has real expected visual effect that the
208+
/// dialect cannot honor, so it must not be silently dropped either.
172209
fn collect_scenes_and_fonts(
173210
parent: &Handle,
174211
scenes: &mut Vec<Value>,
@@ -182,6 +219,12 @@ fn collect_scenes_and_fonts(
182219
// Recurse: html5ever may nest siblings inside the <font> element.
183220
collect_scenes_and_fonts(child, scenes, fonts)?;
184221
}
222+
Some("style") => return Err(HtmlError::StyleElementUnsupported),
223+
Some(other) if find_element(child, "scene").is_some() => {
224+
return Err(HtmlError::NestedScene {
225+
parent: other.to_string(),
226+
});
227+
}
185228
_ => {}
186229
}
187230
}

‎crates/rustmotion-html/src/style.rs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,22 @@ use serde_json::{Map, Value};
22

33
use crate::HtmlError;
44

5-
/// Coerce a CSS value string into JSON. A bare number or `<n>px` becomes a JSON
6-
/// number (integral → integer, so it deserializes into `u32`/`f32` fields);
7-
/// everything else (`%`, `auto`, `fr`, colors, keywords) stays a string.
5+
/// Coerce a CSS value string into JSON. `true`/`false` become a JSON boolean
6+
/// (aligned with [`coerce_dsl_value`] — without this, no `bool` schema field
7+
/// is reachable from HTML: `auto_scroll`, `diff`, `loop`, `show_grid`,
8+
/// `show_borders`, `pulse`, … all reject the JSON string `"true"`/`"false"`
9+
/// that a naive coercion would otherwise produce). A bare number or `<n>px`
10+
/// becomes a JSON number (integral → integer, so it deserializes into
11+
/// `u32`/`f32` fields); everything else (`%`, `auto`, `fr`, colors, keywords)
12+
/// stays a string.
813
pub fn coerce_value(raw: &str) -> Value {
914
let t = raw.trim();
15+
if t == "true" {
16+
return Value::Bool(true);
17+
}
18+
if t == "false" {
19+
return Value::Bool(false);
20+
}
1021
let num = t.strip_suffix("px").unwrap_or(t).trim();
1122
if let Ok(f) = num.parse::<f64>() {
1223
if f.fract() == 0.0 && f.abs() < 9_007_199_254_740_992.0 {
@@ -167,6 +178,13 @@ mod tests {
167178
assert_eq!(coerce_value("1fr"), json!("1fr"));
168179
}
169180

181+
#[test]
182+
fn coerce_value_true_false_become_json_booleans() {
183+
assert_eq!(coerce_value("true"), json!(true));
184+
assert_eq!(coerce_value("false"), json!(false));
185+
assert_eq!(coerce_value(" true "), json!(true), "trims whitespace too");
186+
}
187+
170188
#[test]
171189
fn parses_declarations_into_style_object() {
172190
let m = parse_inline_style("font-size:96px; color:#fff; text-align:center");

0 commit comments

Comments
 (0)