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

@@ -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<Library> {
+/// 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<Library> {
     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<Library> {
             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<Result<Library>> {
         for (name, value) in &entities {
             element = element.replace(name.as_str(), value);
         }
-        parse_library(&format!("<svg>{element}</svg>"), C)
+        // The SVG keeps only the text variables, so the values of the other kinds aren't read.
+        parse_library(&format!("<svg>{element}</svg>"), C, true)
     };
     Some(read())
 }
@@ -640,7 +645,7 @@ pub(super) fn load_library(s: &mut Session, p: &Value) -> Result<Value> {
     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<String> = vars.variables.iter().filter(|v| !kept(v)).map(|v| v.name.clone()).collect();

crates/engine/src/tests/variables.rs

@@ -3304,8 +3304,8 @@ fn an_svg_without_readable_variable_data_opens_its_art() {
     let art = "<rect x=\"1\" y=\"1\" width=\"5\" height=\"5\"/></svg>";
     let untraited = format!("{head}<variableSets><variableSet><variables><variable varName=\"A\"/></variables></variableSet></variableSets>{art}");
     let relative = format!(
-        "{head}<variableSets><variableSet><variables><variable varName=\"A\" trait=\"fileref\"/></variables>\
-         <sampleDataSets><sampleDataSet dataSetName=\"D\"><A>a.png</A></sampleDataSet></sampleDataSets></variableSet></variableSets>{art}"
+        "{head}<variableSets><variableSet><variables><variable varName=\"T\" trait=\"textcontent\"/><variable varName=\"A\" trait=\"fileref\"/></variables>\
+         <sampleDataSets><sampleDataSet dataSetName=\"D\"><T><p>kept</p></T><A>a.png</A></sampleDataSet></sampleDataSets></variableSet></variableSets>{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::<Vec<_>>(), ["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");
 }