XLSX: a kept VML drawing drops a shape as a note only when its client data's ObjectType is Note, so a control whose caption or other attribute reads like one is kept

crates/xlsx/src/controls.rs

@@ -414,10 +414,18 @@ pub struct ReadControls {
 /// has client data that is neither a note (GridCraft writes notes) nor an embedded object's
 /// picture (`Pict`, which means nothing without its sheet entry).
 fn unlisted_kept(shape: &El) -> bool {
-    shape
-        .child("ClientData")
-        .and_then(|d| d.attr("ObjectType"))
-        .is_some_and(|t| !t.trim().eq_ignore_ascii_case("note") && !t.trim().eq_ignore_ascii_case("pict"))
+    object_type_of(shape).is_some_and(|t| !t.eq_ignore_ascii_case("note") && !t.eq_ignore_ascii_case("pict"))
+}
+
+/// A VML shape's client data `ObjectType`, trimmed (`Note`, `Button`, `Checkbox`).
+fn object_type_of(shape: &El) -> Option<&str> {
+    shape.child("ClientData").and_then(|d| d.attr("ObjectType")).map(str::trim)
+}
+
+/// Whether a VML shape's client data is a note's: its `ObjectType` is `Note` in any case and with
+/// spaces around it. Only that attribute counts, not text elsewhere in the shape (a caption).
+pub(crate) fn is_note_data(data: &El) -> bool {
+    data.attr("ObjectType").is_some_and(|t| t.trim().eq_ignore_ascii_case("note"))
 }
 
 /// Reads a sheet's buttons, scroll bars and spin buttons: each `<control>` with its properties

crates/xlsx/src/passthrough/read.rs

@@ -577,26 +577,18 @@ fn shape_spans(vml: &str) -> Vec<(usize, usize)> {
     spans
 }
 
-/// Whether a VML shape is a note: its client data's `ObjectType` is `Note` in any case, in either
-/// quote and with spaces around it, as the control reader reads it.
-fn is_note_shape(shape: &str) -> bool {
-    shape.match_indices("ObjectType").any(|(i, m)| {
-        let Some(rest) = shape.get(i + m.len()..).map(str::trim_start).and_then(|r| r.strip_prefix('=')).map(str::trim_start) else {
-            return false;
-        };
-        let Some(q) = rest.chars().next().filter(|c| matches!(c, '"' | '\'')) else { return false };
-        rest.get(1..).and_then(|v| v.split(q).next()).is_some_and(|v| v.trim().eq_ignore_ascii_case("note"))
-    })
-}
-
-/// VML without its note shapes (see [`is_note_shape`]) and the shapes numbered in `drop`.
+/// VML without its note shapes and the shapes numbered in `drop`. A note is told as the control
+/// reader tells it, from the shape's client data tag and never from text elsewhere in the shape:
+/// its `ObjectType` ([`crate::controls::is_note_data`]). The tag is read on its own
+/// ([`xml::first_start`]), so a caption's unclosed `<br>` doesn't hide it.
 fn strip_shapes(vml: &str, drop: &HashSet<u64>) -> String {
     let mut out = String::with_capacity(vml.len());
     let mut last = 0;
     for (a, b) in shape_spans(vml) {
         let shape = vml.get(a..b).unwrap_or("");
+        let note = xml::first_start(shape.as_bytes(), "ClientData").is_some_and(|d| crate::controls::is_note_data(&d));
         let numbered = shape_numbers(shape).first().is_some_and(|n| drop.contains(n));
-        if is_note_shape(shape) || numbered {
+        if note || numbered {
             out.push_str(vml.get(last..a).unwrap_or(""));
             last = b;
         }

crates/xlsx/src/tests/controls_tests.rs

@@ -735,9 +735,66 @@ fn a_note_spelled_in_another_case_or_quote_is_not_kept_beside_the_written_one()
         let (wb, report) = read_xlsx(&vml_only_others(false, true, Some(attr))).unwrap();
         assert_eq!(wb.sheet(0).unwrap().comments.len(), 1, "{attr}: {:?}", report.warnings);
         let out = write_xlsx(&wb).unwrap();
-        let vml = kept_vml_only_others(&out).to_ascii_lowercase().replace(['\'', ' '], "\"");
-        assert_eq!(vml.matches("objecttype=\"note\"").count() + vml.matches("objecttype=\"\"note\"\"").count(), 1, "{attr}: {vml}");
+        let vml = kept_vml_only_others(&out);
+        assert_eq!(note_shapes(&vml), 1, "{attr}: {vml}");
         let (back, _) = read_xlsx(&out).unwrap();
         assert_eq!(back.sheet(0).unwrap().comments.len(), 1, "{attr}");
     }
 }
+
+/// The note shapes in VML text, counted as the reader tells a note: by each shape's client data
+/// `ObjectType`, whatever its spelling.
+fn note_shapes(vml: &str) -> usize {
+    crate::xml::parse(vml.as_bytes()).unwrap().kids("shape").filter(|s| s.child("ClientData").is_some_and(crate::controls::is_note_data)).count()
+}
+
+/// The saved package's one VML drawing.
+fn only_vml(out: &[u8]) -> String {
+    let mut pkg = Package::open(out).unwrap();
+    let names: Vec<String> = pkg.names().into_iter().filter(|n| n.ends_with(".vml")).collect();
+    assert_eq!(names.len(), 1, "{names:?}");
+    text(&mut pkg, &names[0])
+}
+
+/// A sheet's VML drawing, as the only part beside the sheet's relationships.
+fn with_vml(shapes: &str) -> Vec<u8> {
+    let rels = r#"<?xml version="1.0"?><Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId9" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/vmlDrawing" Target="../drawings/vmlDrawing1.vml"/></Relationships>"#;
+    let vml = format!(
+        r#"<xml xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel">{shapes}</xml>"#
+    );
+    minimal(
+        r#"<sheetData/><legacyDrawing r:id="rId9"/>"#,
+        &[("xl/worksheets/_rels/sheet1.xml.rels", rels), ("xl/drawings/vmlDrawing1.vml", &vml)],
+        "",
+        "",
+    )
+}
+
+/// Text that reads like a note's `ObjectType` in a check box's caption or in another attribute of
+/// an option button doesn't make either a note: both are kept, and a modelled button with that
+/// caption keeps it.
+#[test]
+fn a_control_whose_text_reads_like_a_note_object_type_is_kept() {
+    let bytes = with_vml(concat!(
+        r##"<v:shape id="_x0000_s1025" type="#_x0000_t201"><v:textbox><div>ObjectType="Note"</div></v:textbox><x:ClientData ObjectType="Button"><x:Anchor>2, 0, 3, 0, 4, 0, 5, 0</x:Anchor></x:ClientData></v:shape>"##,
+        r##"<v:shape id="_x0000_s1026" type="#_x0000_t201"><v:textbox><div>ObjectType="Note"</div></v:textbox><x:ClientData ObjectType="Checkbox"><x:Anchor>2, 0, 7, 0, 4, 0, 8, 0</x:Anchor><x:FmlaLink>$B$7</x:FmlaLink></x:ClientData></v:shape>"##,
+        r##"<v:shape id="_x0000_s1027" type="#_x0000_t201" alt="ObjectType='note'"><x:ClientData ObjectType="Radio"><x:Anchor>2, 0, 9, 0, 4, 0, 10, 0</x:Anchor></x:ClientData></v:shape>"##,
+    ));
+    let (wb, report) = read_xlsx(&bytes).unwrap();
+    let sh = wb.sheet(0).unwrap();
+    assert_eq!(sh.controls.len(), 1, "{:?}", report.warnings);
+    assert_eq!(sh.controls[0].text, "ObjectType=\"Note\"");
+    let mut out = write_xlsx(&wb).unwrap();
+    for _ in 0..2 {
+        let vml = only_vml(&out);
+        assert_eq!(vml.matches("ObjectType=\"Checkbox\"").count(), 1, "the check box is kept: {vml}");
+        assert_eq!(vml.matches("ObjectType=\"Radio\"").count(), 1, "the option button is kept: {vml}");
+        assert_eq!(vml.matches("ObjectType=\"Button\"").count(), 1, "the button once: {vml}");
+        assert_eq!(note_shapes(&vml), 0, "{vml}");
+        let (back, _) = read_xlsx(&out).unwrap();
+        let sh = back.sheet(0).unwrap();
+        assert_eq!(sh.controls.len(), 1);
+        assert_eq!(sh.controls[0].text, "ObjectType=\"Note\"");
+        out = write_xlsx(&back).unwrap();
+    }
+}

crates/xlsx/src/xml.rs

@@ -213,6 +213,29 @@ pub fn parse(xml: &[u8]) -> Result<El, IoError> {
     parse_streaming(xml, &[], &mut |_| Ok(()))
 }
 
+/// The first element named `local_name` (namespace prefix dropped) in `xml`, read from its start tag
+/// alone: no children, and nothing after the tag is read, so unbalanced markup around it (a VML
+/// caption's unclosed `<br>`) neither hides nor moves it. `None` when there is none, or when the
+/// markup before it is malformed.
+pub fn first_start(xml: &[u8], local_name: &str) -> Option<El> {
+    let mut reader = Reader::from_reader(xml);
+    {
+        let cfg = reader.config_mut();
+        cfg.check_end_names = false;
+        cfg.allow_unmatched_ends = true;
+        cfg.allow_dangling_amp = true;
+    }
+    let mut buf = Vec::new();
+    loop {
+        match reader.read_event_into(&mut buf).ok()? {
+            Event::Start(e) | Event::Empty(e) if local(e.name().as_ref()) == local_name => return Some(start_el(&e)),
+            Event::Eof => return None,
+            _ => {}
+        }
+        buf.clear();
+    }
+}
+
 /// Parses a part; complete elements named in `split` that are children or grandchildren of the
 /// root (`sst/si`, `worksheet/sheetData/row`) are handed to `cb` and not kept in the returned
 /// tree. Deeper elements of the same name (the `xdr:row` of a form control's anchor) stay in the