Merge pull request #105 from skylartaylor/fix/idml-scalar-inset-spacing

IDML: import scalar InsetSpacing property values

crates/idml/src/import.rs

@@ -2244,13 +2244,8 @@ fn text_frame_options(e: &El) -> TextFrameOptions {
         o.column_width = v;
     }
     o.balance_columns = e.get("VerticalBalanceColumns") == Some("true");
-    if let Some(l) = e.prop_el("InsetSpacing") {
-        let v: Vec<f64> = l.find_all("ListItem").filter_map(|i| i.text_content().trim().parse().ok()).collect();
-        if v.len() == 4 {
-            o.inset = [v[0], v[1], v[2], v[3]];
-        }
-    } else if let Some(v) = e.num("InsetSpacing") {
-        o.inset = [v; 4];
+    if let Some(inset) = inset_spacing(e) {
+        o.inset = inset;
     }
     if let Some(v) = e.get("VerticalJustification") {
         o.vertical_justification = names::vj_in(v);
@@ -2274,6 +2269,32 @@ fn text_frame_options(e: &El) -> TextFrameOptions {
     o
 }
 
+/// A scalar applies to every edge; a list is top, left, bottom, right. Keep the
+/// existing property-element precedence over the legacy scalar attribute, even
+/// when the property is malformed. Finite negative insets remain supported.
+fn inset_spacing(e: &El) -> Option<[f64; 4]> {
+    let number = |s: &str| s.trim().parse::<f64>().ok().filter(|v| v.is_finite());
+    let Some(property) = e.prop_el("InsetSpacing") else {
+        return e.get("InsetSpacing").and_then(number).map(|v| [v; 4]);
+    };
+    let mut items = property.elements();
+    if property.get("type") == Some("list") || property.find("ListItem").is_some() {
+        let mut inset = [0.0; 4];
+        for edge in &mut inset {
+            let item = items.next()?;
+            if item.local() != "ListItem" || item.elements().next().is_some() {
+                return None;
+            }
+            *edge = number(&item.text_content())?;
+        }
+        items.next().is_none().then_some(inset)
+    } else if items.next().is_none() {
+        number(&property.text_content()).map(|v| [v; 4])
+    } else {
+        None
+    }
+}
+
 fn path_of(pg: &El) -> PathData {
     let mut subs = Vec::new();
     for g in pg.find_all("GeometryPathType") {

crates/idml/src/tests.rs

@@ -269,6 +269,119 @@ fn imports_hand_written_fixture() {
     assert_eq!(a.name, "photo.jpg");
 }
 
+/// Hand-written inset encodings, independent of the exporter's four-item list.
+fn inset_fixture(preference: &str, object_styles: &str) -> Document {
+    let designmap = format!(
+        r#"<Document xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging" Self="d">
+          <RootObjectStyleGroup Self="ro">{object_styles}</RootObjectStyleGroup>
+          <idPkg:Spread src="Spreads/Spread_s.xml"/>
+        </Document>"#
+    );
+    let spread = format!(
+        r#"<idPkg:Spread xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging">
+          <Spread Self="s">
+            <Page Self="p" GeometricBounds="0 0 100 100" ItemTransform="1 0 0 1 0 0"/>
+            <TextFrame Self="f" AppliedObjectStyle="ObjectStyle/Padded">
+              <Properties><PathGeometry><GeometryPathType PathOpen="false"><PathPointArray>
+                <PathPointType Anchor="0 0"/><PathPointType Anchor="0 100"/>
+                <PathPointType Anchor="100 100"/><PathPointType Anchor="100 0"/>
+              </PathPointArray></GeometryPathType></PathGeometry></Properties>
+              {preference}
+            </TextFrame>
+          </Spread>
+        </idPkg:Spread>"#
+    );
+    import_idml(&zip_files(&[("designmap.xml", &designmap), ("Spreads/Spread_s.xml", &spread)])).unwrap()
+}
+
+fn frame_inset(d: &Document) -> [f64; 4] {
+    d.spreads[0].items[0].text_frame().unwrap().options.inset
+}
+
+fn inset_list(values: &[&str]) -> String {
+    let items = values.iter().map(|v| format!(r#"<ListItem type="unit">{v}</ListItem>"#)).collect::<String>();
+    format!(r#"<Properties><InsetSpacing type="list">{items}</InsetSpacing></Properties>"#)
+}
+
+#[test]
+fn imports_scalar_inset_properties_like_lists_and_attributes() {
+    let scalar =
+        inset_fixture(r#"<TextFramePreference><Properties><InsetSpacing type="unit"> 9 </InsetSpacing></Properties></TextFramePreference>"#, "");
+    let list = inset_fixture(&format!("<TextFramePreference>{}</TextFramePreference>", inset_list(&["9"; 4])), "");
+    let attribute = inset_fixture(r#"<TextFramePreference InsetSpacing="9"/>"#, "");
+    let untyped = inset_fixture(r#"<TextFramePreference><Properties><InsetSpacing>9</InsetSpacing></Properties></TextFramePreference>"#, "");
+    assert_eq!(frame_inset(&scalar), [9.0; 4]);
+    assert_eq!(frame_inset(&scalar), frame_inset(&list));
+    assert_eq!(frame_inset(&scalar), frame_inset(&attribute));
+    assert_eq!(frame_inset(&scalar), frame_inset(&untyped));
+    assert_eq!(scalar.spreads[0].items[0].text_area(), Rect::new(9.0, 9.0, 91.0, 91.0));
+    let round_trip = import_idml(&export_idml(&scalar)).unwrap();
+    assert_eq!(frame_inset(&round_trip), [9.0; 4]);
+}
+
+#[test]
+fn inset_property_keeps_precedence_over_attribute() {
+    for (property, expected) in [
+        (r#"<Properties><InsetSpacing type="unit">9</InsetSpacing></Properties>"#.to_string(), [9.0; 4]),
+        (inset_list(&["1", "2", "3", "4"]), [1.0, 2.0, 3.0, 4.0]),
+        (inset_list(&["1", "2", "3"]), [0.0; 4]),
+        (r#"<Properties><InsetSpacing type="unit">invalid</InsetSpacing></Properties>"#.to_string(), [0.0; 4]),
+    ] {
+        let d = inset_fixture(&format!(r#"<TextFramePreference InsetSpacing="42">{property}</TextFramePreference>"#), "");
+        assert_eq!(frame_inset(&d), expected, "{property}");
+    }
+}
+
+#[test]
+fn inset_spacing_rejects_malformed_and_nonfinite_values() {
+    let mut preferences = vec![
+        String::new(),
+        "<TextFramePreference/>".to_string(),
+        r#"<TextFramePreference><Properties><InsetSpacing type="list">9</InsetSpacing></Properties></TextFramePreference>"#.to_string(),
+        r#"<TextFramePreference><Properties><InsetSpacing type="unit"><Other>9</Other></InsetSpacing></Properties></TextFramePreference>"#
+            .to_string(),
+    ];
+    for value in ["", "invalid", "NaN", "inf", "-inf", "1e309"] {
+        preferences.push(format!(r#"<TextFramePreference InsetSpacing="{value}"/>"#));
+        preferences
+            .push(format!(r#"<TextFramePreference><Properties><InsetSpacing type="unit">{value}</InsetSpacing></Properties></TextFramePreference>"#));
+        preferences.push(format!("<TextFramePreference>{}</TextFramePreference>", inset_list(&["1", value, "3", "4"])));
+    }
+    for values in [&[][..], &["9"][..], &["1", "2", "3"][..], &["1", "2", "3", "4", "5"][..], &["1", "bad", "2", "3", "4"][..]] {
+        preferences.push(format!("<TextFramePreference>{}</TextFramePreference>", inset_list(values)));
+    }
+    preferences.push(format!("<TextFramePreference>{}</TextFramePreference>", inset_list(&["1", "<Other>2</Other>", "3", "4"])));
+    for preference in preferences {
+        assert_eq!(frame_inset(&inset_fixture(&preference, "")), [0.0; 4], "{preference}");
+    }
+}
+
+#[test]
+fn inset_spacing_preserves_finite_signed_values_and_edge_order() {
+    for value in ["0", "-2.5", " 4.5 "] {
+        let expected = [value.trim().parse::<f64>().unwrap(); 4];
+        for preference in [
+            format!(r#"<TextFramePreference InsetSpacing="{value}"/>"#),
+            format!(r#"<TextFramePreference><Properties><InsetSpacing type="unit">{value}</InsetSpacing></Properties></TextFramePreference>"#),
+            format!("<TextFramePreference>{}</TextFramePreference>", inset_list(&[value; 4])),
+        ] {
+            assert_eq!(frame_inset(&inset_fixture(&preference, "")), expected, "{preference}");
+        }
+    }
+    let d = inset_fixture(&format!("<TextFramePreference>{}</TextFramePreference>", inset_list(&["1", "-2", "3.5", "4"])), "");
+    assert_eq!(frame_inset(&d), [1.0, -2.0, 3.5, 4.0], "top, left, bottom, right");
+}
+
+#[test]
+fn imports_scalar_insets_in_object_style_without_replacing_frame_override() {
+    let styles = r#"<ObjectStyle Self="ObjectStyle/Padded" Name="Padded" EnableTextFrameGeneralOptions="true">
+      <TextFramePreference><Properties><InsetSpacing type="unit">9</InsetSpacing></Properties></TextFramePreference>
+    </ObjectStyle>"#;
+    let d = inset_fixture(r#"<TextFramePreference InsetSpacing="3"/>"#, styles);
+    assert_eq!(d.styles.object.iter().find(|s| s.name == "Padded").unwrap().text_frame.as_ref().unwrap().inset, [9.0; 4]);
+    assert_eq!(frame_inset(&d), [3.0; 4]);
+}
+
 #[test]
 fn rejects_non_idml() {
     assert!(import_idml(b"not a zip").is_err());