From 5725f7b44e4752e7d1268413d8ba09ddc3be5f09 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 14:44:00 -0500 Subject: [PATCH] XLSX: a kept VML drawing drops a note whatever the case or quotes of its ObjectType, so a comment gets one note --- crates/xlsx/src/passthrough/read.rs | 16 ++++++++++-- crates/xlsx/src/tests/controls_tests.rs | 34 ++++++++++++++++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/crates/xlsx/src/passthrough/read.rs b/crates/xlsx/src/passthrough/read.rs index 73c176136..7ad0f8867 100644 --- a/crates/xlsx/src/passthrough/read.rs +++ b/crates/xlsx/src/passthrough/read.rs @@ -577,14 +577,26 @@ fn shape_spans(vml: &str) -> Vec<(usize, usize)> { spans } -/// VML without its note shapes (`ObjectType="Note"`) and the shapes numbered in `drop`. +/// 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`. 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 numbered = shape_numbers(shape).first().is_some_and(|n| drop.contains(n)); - if shape.contains("ObjectType=\"Note\"") || numbered { + if is_note_shape(shape) || 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 4fbb65218..8936e0153 100644 --- a/crates/xlsx/src/tests/controls_tests.rs +++ b/crates/xlsx/src/tests/controls_tests.rs @@ -577,15 +577,16 @@ fn controls_naming_one_properties_part_read_it_once() { /// A legacy VML drawing with a button (`1025`, modelled), a check box (`1026`) and an option /// button (`1027`, with a picture through the drawing's own relationship) that only the drawing -/// lists, plus `note` (a note shape for A1, with its comments part) when set. `listed`: the sheet's +/// lists, plus `note` (a note shape for A1 whose client data carries that `ObjectType` attribute, +/// with its comments part) when given. `listed`: the sheet's /// `` lists the button. -fn vml_only_others(listed: bool, button: bool, note: bool) -> Vec { +fn vml_only_others(listed: bool, button: bool, note: Option<&str>) -> Vec { const REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; let mut rels = format!(r#""#); if listed { rels.push_str(&format!(r#""#)); } - if note { + if note.is_some() { rels.push_str(&format!(r#""#)); } let rels = @@ -595,8 +596,8 @@ fn vml_only_others(listed: bool, button: bool, note: bool) -> Vec { ); let prop_a = r#""#; let mut shapes = String::new(); - if note { - shapes.push_str(r##"1, 15, 0, 2, 3, 15, 4, 400"##); + if let Some(attr) = note { + shapes.push_str(&format!(r##"1, 15, 0, 2, 3, 15, 4, 400"##)); } if button { shapes.push_str(r##"
Go
2, 0, 3, 0, 4, 0, 5, 0[0]!Go"##); @@ -621,7 +622,7 @@ fn vml_only_others(listed: bool, button: bool, note: bool) -> Vec { if listed { parts.push(("xl/ctrlProps/a.xml", prop_a)); } - if note { + if note.is_some() { parts.push(("xl/comments1.xml", notes)); } // As `minimal` builds it, with content types for the picture and the drawing. @@ -670,7 +671,7 @@ fn kept_vml_only_others(out: &[u8]) -> String { /// kept: the save writes them beside the regenerated button, and they survive a second round trip. #[test] fn vml_only_controls_survive_when_the_last_listed_control_is_modelled() { - let bytes = vml_only_others(true, true, false); + let bytes = vml_only_others(true, true, None); let (wb, report) = read_xlsx(&bytes).unwrap(); let sh = wb.sheet(0).unwrap(); assert_eq!(sh.controls.len(), 1, "the button is modelled"); @@ -694,7 +695,7 @@ fn vml_only_controls_survive_when_the_last_listed_control_is_modelled() { /// lists (no ``, no notes, nothing modelled): the drawing is kept on its own. #[test] fn a_sheet_whose_only_controls_are_vml_only_and_unmodelled_keeps_them() { - let bytes = vml_only_others(false, false, false); + let bytes = vml_only_others(false, false, None); let (wb, report) = read_xlsx(&bytes).unwrap(); assert!(wb.sheet(0).unwrap().controls.is_empty()); let out = write_xlsx(&wb).unwrap(); @@ -711,7 +712,7 @@ fn a_sheet_whose_only_controls_are_vml_only_and_unmodelled_keeps_them() { /// option button stay, all in the one drawing. #[test] fn vml_only_controls_beside_a_note_are_kept() { - let bytes = vml_only_others(false, true, true); + let bytes = vml_only_others(false, true, Some(r#"ObjectType="Note""#)); let (wb, report) = read_xlsx(&bytes).unwrap(); let sh = wb.sheet(0).unwrap(); assert_eq!((sh.controls.len(), sh.comments.len()), (1, 1), "{:?}", report.warnings); @@ -725,3 +726,18 @@ fn vml_only_controls_beside_a_note_are_kept() { let vml = kept_vml_only_others(&write_xlsx(&back).unwrap()); assert_eq!(vml.matches("ObjectType=\"Note\"").count(), 1, "{vml}"); } + +#[test] +fn a_note_spelled_in_another_case_or_quote_is_not_kept_beside_the_written_one() { + // The note's client data reads ObjectType='note': still a note, so the kept drawing drops it + // and the note GridCraft writes for the comment is the only one. + for attr in ["ObjectType='note'", "ObjectType = \"NOTE\"", "ObjectType=\" Note \""] { + 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 (back, _) = read_xlsx(&out).unwrap(); + assert_eq!(back.sheet(0).unwrap().comments.len(), 1, "{attr}"); + } +}