From c6e65c3968ea3ba8c0daab1f1e034b66435d2f17 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 14:10:15 -0800 Subject: [PATCH] Variables: SVG variable data reads only the text values of its library An SVG keeps only its text variables, so the values of its visibility, image and graph variables are passed over unread, and one that Load Variable Library would refuse no longer keeps the text variables from opening. --- crates/engine/src/cmd/variablecmds/library.rs | 15 ++++++++---- crates/engine/src/tests/variables.rs | 24 ++++++++++++------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index 1c20fc77f7..2392a80ee4 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -235,9 +235,10 @@ impl Library { /// A variable's kind comes from its `trait` ([`trait_kind`]). In a data set, an element named after /// a variable gives its value, as [`library_value`] reads it. A variable or data set declared /// twice, a value for a variable the library doesn't declare, a value that doesn't fit its -/// variable's kind, and a library past the caps are refused. Entities that its document type -/// declaration defines are read as [`without_doctype`] says. -fn parse_library(xml: &str, cmd: &str) -> Result { +/// variable's kind, and a library past the caps are refused. With `text_only`, only text values +/// are read, and the values of the other kinds are passed over unread. Entities that its document +/// type declaration defines are read as [`without_doctype`] says. +fn parse_library(xml: &str, cmd: &str, text_only: bool) -> Result { let xml = without_doctype(xml, cmd)?; // roxmltree parses nested elements recursively. if crate::cmd::place::docx::nesting(&xml) > MAX_LIBRARY_NESTING { @@ -294,6 +295,9 @@ fn parse_library(xml: &str, cmd: &str) -> Result { if !held.insert(var) { return Err(bad(cmd, format!("data set `{name}` holds two values for `{var}`"))); } + if text_only && *kind != VariableKind::Text { + continue; + } if let Some(value) = library_value(el, *kind, &name, cmd)? { values.insert(var.to_string(), value); } @@ -522,7 +526,8 @@ pub(crate) fn svg_library(svg: &str) -> Option> { for (name, value) in &entities { element = element.replace(name.as_str(), value); } - parse_library(&format!("{element}"), C) + // The SVG keeps only the text variables, so the values of the other kinds aren't read. + parse_library(&format!("{element}"), C, true) }; Some(read()) } @@ -640,7 +645,7 @@ pub(super) fn load_library(s: &mut Session, p: &Value) -> Result { const C: &str = "variable.loadLibrary"; let bytes = library_bytes(p, C)?; let xml = std::str::from_utf8(bytes.strip_prefix(b"\xEF\xBB\xBF").unwrap_or(&bytes)).map_err(|_| bad(C, "the variable library is not UTF-8"))?; - let lib = parse_library(xml, C)?; + let lib = parse_library(xml, C, false)?; let vars = &s.doc()?.doc.variables; let kept = |v: &Variable| lib.variables.iter().any(|l| l.name == v.name && l.kind == v.kind); let deleted: Vec = vars.variables.iter().filter(|v| !kept(v)).map(|v| v.name.clone()).collect(); diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index 64d5923b9c..bb5b73b2f2 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -3304,8 +3304,8 @@ fn an_svg_without_readable_variable_data_opens_its_art() { let art = ""; let untraited = format!("{head}{art}"); let relative = format!( - "{head}\ - a.png{art}" + "{head}\ +

kept

a.png{art}" ); let refs = "&ns_big;".repeat(40_000); let big = "x".repeat(250); @@ -3367,12 +3367,18 @@ fn an_svg_without_readable_variable_data_opens_its_art() { let (o, warnings) = svg_opened(&untraited, "untraited.svg"); assert!(warnings.contains(&json!("variables that aren't text were left out: unknown (1)")), "{warnings:?}"); assert!(o.doc().unwrap().doc.variables.variables.is_empty()); + // The values of the other kinds aren't read, so one that Load Variable Library would refuse + // leaves the text variables and their values as they open. + let (o, warnings) = svg_opened(&relative, "relative.svg"); + assert!(warnings.contains(&json!("variables that aren't text were left out: fileref (1)")), "{warnings:?}"); + let vars = &o.doc().unwrap().doc.variables; + assert_eq!(vars.variables.iter().map(|v| v.name.as_str()).collect::>(), ["T"]); + assert_eq!(vars.dataset("D").map(|d| d.values.clone()), Some([("T".to_string(), vectorcraft_doc::DataValue::Text("kept".into()))].into())); - for (svg, why) in [(relative, "that isn't a file's absolute path or `file:` URL"), (growing, "entities expand to more than")] { - let (o, warnings) = svg_opened(&svg, "broken.svg"); - assert!(warnings.iter().any(|w| w.as_str().is_some_and(|w| w.contains("variable data wasn't read") && w.contains(why))), "{warnings:?}"); - let d = &o.doc().unwrap().doc; - assert!(d.variables.variables.is_empty()); - assert_eq!(vectorcraft_testkit::svg_shapes(&vectorcraft_svg::export(d, &Default::default())), 1, "the art opens"); - } + let (o, warnings) = svg_opened(&growing, "broken.svg"); + let why = "entities expand to more than"; + assert!(warnings.iter().any(|w| w.as_str().is_some_and(|w| w.contains("variable data wasn't read") && w.contains(why))), "{warnings:?}"); + let d = &o.doc().unwrap().doc; + assert!(d.variables.variables.is_empty()); + assert_eq!(vectorcraft_testkit::svg_shapes(&vectorcraft_svg::export(d, &Default::default())), 1, "the art opens"); }