From 92117e9a19f1a8045e3dfc5648f8132c761cca03 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 11:06:11 +0200 Subject: [PATCH] fix(html): preserve the document around the rustmotion element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four write-back entry points (`set_inline_style`, `set_text_content`, `set_attribute`, `remove_inline_style`) re-serialize only the `` element, and rustmotion-studio writes that string straight over the source file (editor/inspector.rs:2168 -> `write_and_note(path, &updated)`). Executed: a 281-byte document with ``, ``, a leading `` comment and a trailing comment becomes 150 bytes — doctype, head, meta, body and both outer comments gone — after one font-size tweak in the inspector. The transpiler happily accepts such documents (`find_element` is a document-wide DFS), so this is a supported authoring form being destroyed. The docstring's "formatting is normalized" does not cover deleting the surrounding document. Secondarily, `serialize_element` (lib.rs:488-497) does `let _ = serialize(...)` and `String::from_utf8(buf).unwrap_or_default()`, so a serializer failure would write an empty file rather than surface an error. Refs #220 --- crates/rustmotion-html/src/lib.rs | 41 ++++++++++-- crates/rustmotion-html/tests/audit_ws_f.rs | 76 ++++++++++++++++++++++ 2 files changed, 110 insertions(+), 7 deletions(-) diff --git a/crates/rustmotion-html/src/lib.rs b/crates/rustmotion-html/src/lib.rs index 69d9ea43..a5a788bb 100644 --- a/crates/rustmotion-html/src/lib.rs +++ b/crates/rustmotion-html/src/lib.rs @@ -401,7 +401,7 @@ pub fn set_inline_style(html: &str, pointer: &str, prop: &str, value: &str) -> O let root = find_element(&dom.document, "rustmotion")?; let target = resolve_pointer(&root, pointer)?; set_style_attr(&target, prop, value)?; - Some(serialize_element(&root)) + splice_rustmotion_subtree(html, &root) } /// Replace the text content of the element addressed by the JSON pointer with @@ -413,7 +413,7 @@ pub fn set_text_content(html: &str, pointer: &str, text: &str) -> Option let root = find_element(&dom.document, "rustmotion")?; let target = resolve_pointer(&root, pointer)?; set_text(&target, text)?; - Some(serialize_element(&root)) + splice_rustmotion_subtree(html, &root) } /// Set or replace a plain attribute on the element addressed by the JSON @@ -429,7 +429,7 @@ pub fn set_attribute(html: &str, pointer: &str, name: &str, value: &str) -> Opti let root = find_element(&dom.document, "rustmotion")?; let target = resolve_pointer(&root, pointer)?; set_attr(&target, name, value)?; - Some(serialize_element(&root)) + splice_rustmotion_subtree(html, &root) } /// Remove one inline `style` property from the element addressed by the JSON @@ -441,7 +441,7 @@ pub fn remove_inline_style(html: &str, pointer: &str, prop: &str) -> Option String { .join("; ") } -fn serialize_element(handle: &Handle) -> String { +/// Re-serialize `root` (the mutated `` subtree) and splice it +/// back into `original` at the exact byte span its `... +/// ` element occupies there, leaving everything before and +/// after — doctype, ``, comments, anything else the author wrote — +/// byte-for-byte untouched. `original` is used as the source of truth for +/// that surrounding content rather than `root`'s own parsed document, +/// because `parse_fragment_dom` parses in a body-fragment context, which +/// does not retain a doctype at all and does not guarantee round-tripping +/// ``/``/`` the way the author wrote them. Refuses +/// (`None`) rather than write a corrupted file when the span can't be +/// located (no ``/`` literal in `original`, e.g. an +/// unclosed root) or the serializer itself fails. +fn splice_rustmotion_subtree(original: &str, root: &Handle) -> Option { + let open_start = original.find("')? + 1; + if close_end <= open_start { + return None; + } + let serialized = serialize_element(root).ok()?; + let mut out = String::with_capacity(original.len() + serialized.len()); + out.push_str(&original[..open_start]); + out.push_str(&serialized); + out.push_str(&original[close_end..]); + Some(out) +} + +fn serialize_element(handle: &Handle) -> Result { let mut buf = Vec::new(); let node: SerializableHandle = handle.clone().into(); let opts = SerializeOpts { traversal_scope: TraversalScope::IncludeNode, ..Default::default() }; - let _ = serialize(&mut buf, &node, opts); - String::from_utf8(buf).unwrap_or_default() + serialize(&mut buf, &node, opts).map_err(|e| HtmlError::SerializeFailed(e.to_string()))?; + String::from_utf8(buf).map_err(|e| HtmlError::SerializeFailed(e.to_string())) } #[cfg(test)] diff --git a/crates/rustmotion-html/tests/audit_ws_f.rs b/crates/rustmotion-html/tests/audit_ws_f.rs index 26165420..e8ab981a 100644 --- a/crates/rustmotion-html/tests/audit_ws_f.rs +++ b/crates/rustmotion-html/tests/audit_ws_f.rs @@ -263,3 +263,79 @@ fn inline_formatting_tags_still_flatten_into_the_parent_text() { json!("Real bold text") ); } + +// --------------------------------------------------------------------------- +// studio HTML write-back silently deletes everything outside +// in the author's file. +// --------------------------------------------------------------------------- + +#[test] +fn write_back_preserves_doctype_head_and_surrounding_comments() { + let html = concat!( + "\n", + "\n", + "\n", + "", + "

Hi

\n", + "\n", + "\n", + ); + let out = rustmotion_html::set_inline_style(html, "/scenes/0/children/0", "font-size", "120") + .expect("pointer resolves"); + assert!( + out.contains(""), + "doctype must survive: {out}" + ); + assert!( + out.contains(""), + "head must survive: {out}" + ); + assert!( + out.contains(""), + "leading comment must survive: {out}" + ); + assert!( + out.contains(""), + "trailing comment must survive: {out}" + ); + assert!( + out.contains("font-size:120"), + "the edit itself must still apply: {out}" + ); + let v = html_to_scenario_value(&out).expect("round-trips through the transpiler"); + assert_eq!( + v["scenes"][0]["children"][0]["style"]["font-size"], + json!(120) + ); +} + +#[test] +fn write_back_via_set_text_content_also_preserves_surrounding_document() { + let html = concat!( + "\n", + "\n", + "", + "

Hi

\n", + "\n", + ); + let out = rustmotion_html::set_text_content(html, "/scenes/0/children/0", "Bonjour") + .expect("pointer resolves"); + assert!( + out.contains(""), + "doctype must survive: {out}" + ); + assert!(out.contains(""), "got: {out}"); + assert!(out.contains(""), "got: {out}"); + let v = html_to_scenario_value(&out).expect("round-trips through the transpiler"); + assert_eq!(v["scenes"][0]["children"][0]["content"], json!("Bonjour")); +} + +#[test] +fn write_back_refuses_when_the_closing_tag_cannot_be_located_in_source() { + let html = r##"

Hi

"##; + let out = rustmotion_html::set_inline_style(html, "/scenes/0/children/0", "font-size", "120"); + assert!( + out.is_none(), + "must refuse rather than write a file it cannot faithfully reconstruct" + ); +}