From 02df071b4c719d46c939968428d48a25ea30b287 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:18:54 -0500 Subject: [PATCH] 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 | 16 +++++-- crates/xlsx/src/passthrough/read.rs | 20 +++----- crates/xlsx/src/tests/controls_tests.rs | 61 ++++++++++++++++++++++++- crates/xlsx/src/xml.rs | 23 ++++++++++ 4 files changed, 100 insertions(+), 20 deletions(-) diff --git a/crates/xlsx/src/controls.rs b/crates/xlsx/src/controls.rs index bf6d9858c..0a9167f85 100644 --- a/crates/xlsx/src/controls.rs +++ b/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 `` with its properties diff --git a/crates/xlsx/src/passthrough/read.rs b/crates/xlsx/src/passthrough/read.rs index 7ad0f8867..669812597 100644 --- a/crates/xlsx/src/passthrough/read.rs +++ b/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 `
` doesn't hide it. fn strip_shapes(vml: &str, drop: &HashSet) -> 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; } diff --git a/crates/xlsx/src/tests/controls_tests.rs b/crates/xlsx/src/tests/controls_tests.rs index 8936e0153..ec4b66c09 100644 --- a/crates/xlsx/src/tests/controls_tests.rs +++ 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 = 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 { + let rels = r#""#; + let vml = format!( + r#"{shapes}"# + ); + minimal( + r#""#, + &[("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##"
ObjectType="Note"
2, 0, 3, 0, 4, 0, 5, 0"##, + r##"
ObjectType="Note"
2, 0, 7, 0, 4, 0, 8, 0$B$7"##, + r##"2, 0, 9, 0, 4, 0, 10, 0"##, + )); + 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(); + } +} diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index b9097eea2..902d420c4 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -213,6 +213,29 @@ pub fn parse(xml: &[u8]) -> Result { 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 `
`) 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 { + 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