From 5cbc168441baf817fdf9375fbc111c1f177ad79f Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 05:39:45 -0500 Subject: [PATCH] XLSX: a kept control beside a modelled one in the same AlternateContent stays with pass-through without_controls dropped a whole child of a kept element as soon as one control in it was modelled, so an mc:AlternateContent holding a scroll bar (modelled) and a list box (left to pass-through) lost the list box's worksheet entry on save, while its VML shape and properties part stayed. The controls are now taken out one by one, in the Choice and the Fallback branches alike, by the shapeId the control reader reads (an attribute, in either quote style); the wrapper stays while a control remains and goes only when it is empty, and a child with none of ours is kept unchanged. strip_shapes already works per shape, so the kept control's shape stays, and its properties part follows its relationship as before. Test: an_unsupported_control_beside_a_modelled_one_in_one_alternate_content_is_kept (after open and save the scroll bar is written once, the list box is still in in both branches with its VML shape and its properties part, and the file reads back with one modelled control and the kept one). --- crates/xlsx/src/controls.rs | 2 +- crates/xlsx/src/passthrough/read.rs | 69 +++++++++++++++++++------ crates/xlsx/src/tests/controls_tests.rs | 62 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/crates/xlsx/src/controls.rs b/crates/xlsx/src/controls.rs index da22eb127..6f56894cd 100644 --- a/crates/xlsx/src/controls.rs +++ b/crates/xlsx/src/controls.rs @@ -359,7 +359,7 @@ fn build(p: Props, kind: ControlKind, anchor: Anchor, name: Option<&str>, id: u3 /// The `` elements one child of a `` element holds: the child itself, or those /// inside its `mc:AlternateContent` (the Choice, else the Fallback). -pub(crate) fn controls_of_child(k: &El) -> Vec<&El> { +fn controls_of_child(k: &El) -> Vec<&El> { match k.name.as_str() { "control" => vec![k], "AlternateContent" => k diff --git a/crates/xlsx/src/passthrough/read.rs b/crates/xlsx/src/passthrough/read.rs index 4fc55b9c6..4e2264e6a 100644 --- a/crates/xlsx/src/passthrough/read.rs +++ b/crates/xlsx/src/passthrough/read.rs @@ -680,21 +680,14 @@ fn attr_value<'a>(x: &'a str, name: &str) -> Option<&'a str> { } /// A kept `` fragment without the controls whose `shapeId` is in `drop` (they are -/// written from the model). `None` when no control is left. A fragment that doesn't parse is -/// returned unchanged. +/// written from the model): a bare `` child goes when it is one of them, and within an +/// `mc:AlternateContent` child only those controls go, in each of its branches, the wrapper +/// staying while a control remains. `None` when nothing is left. A fragment that doesn't parse +/// is returned unchanged. fn without_controls(frag: &str, drop: &HashSet) -> Option { if drop.is_empty() { return Some(frag.to_string()); } - // A kept child names its shapes the way the control reader reads them (`shapeId` in either - // quote style, through mc:AlternateContent), so what the reader modelled is what is dropped. - let numbered = |x: &str| { - crate::xml::parse(x.as_bytes()).is_ok_and(|e| { - crate::controls::controls_of_child(&e) - .iter() - .any(|c| c.attr("shapeId").and_then(crate::controls::shape_number).is_some_and(|n| drop.contains(&n))) - }) - }; let mut reader = Reader::from_str(frag); let mut out = String::with_capacity(frag.len()); let (mut last, mut left, mut inside) = (0usize, 0usize, false); @@ -727,17 +720,63 @@ fn without_controls(frag: &str, drop: &HashSet) -> Option { continue; } let Ok(end) = usize::try_from(reader.buffer_position()) else { return Some(frag.to_string()) }; - if numbered(frag.get(start..end).unwrap_or("")) { - out.push_str(frag.get(last..start).unwrap_or("")); - last = end; - } else { + let text = frag.get(start..end).unwrap_or(""); + let Some((kept, controls, dropped)) = controls_kept(text, drop) else { return Some(frag.to_string()) }; + if dropped == 0 { + // Nothing of ours in it: the child stays as it is. left += 1; + } else { + out.push_str(frag.get(last..start).unwrap_or("")); + if controls > 0 { + out.push_str(&kept); + left += 1; + } + last = end; } } out.push_str(frag.get(last..).unwrap_or("")); (left > 0).then_some(out) } +/// One child of a `` element with the `` elements numbered in `drop` taken +/// out, wherever they are in it (a bare control, or the Choice and Fallback branches of an +/// `mc:AlternateContent`), as (the text, the controls that remain, the controls taken out). The +/// `shapeId` is read as the control reader reads it (an attribute, in either quote style). `None` +/// when the child doesn't parse. +fn controls_kept(child: &str, drop: &HashSet) -> Option<(String, usize, usize)> { + let mut reader = Reader::from_str(child); + let mut out = String::with_capacity(child.len()); + let (mut last, mut kept, mut dropped) = (0usize, 0usize, 0usize); + loop { + let start = usize::try_from(reader.buffer_position()).ok()?; + let (e, has_end) = match reader.read_event().ok()? { + Event::Eof => break, + Event::Start(e) => (e, true), + Event::Empty(e) => (e, false), + _ => continue, + }; + if e.local_name().as_ref() != b"control" { + // A wrapper or a branch: its controls are the events that follow. + continue; + } + let number = + e.try_get_attribute("shapeId").ok().flatten().and_then(|a| std::str::from_utf8(&a.value).ok().and_then(crate::controls::shape_number)); + if has_end { + reader.read_to_end(e.name()).ok()?; + } + let end = usize::try_from(reader.buffer_position()).ok()?; + if number.is_some_and(|n| drop.contains(&n)) { + out.push_str(child.get(last..start)?); + last = end; + dropped += 1; + } else { + kept += 1; + } + } + out.push_str(child.get(last..)?); + Some((out, kept, dropped)) +} + /// Rewrites relationship-id attributes (`r:id`, `r:embed`…: any attribute in the relationships /// namespace) of an XML fragment through `map`. `None` when the fragment is damaged or `map` has /// no id for one of them. diff --git a/crates/xlsx/src/tests/controls_tests.rs b/crates/xlsx/src/tests/controls_tests.rs index 323ca14f3..46b81428c 100644 --- a/crates/xlsx/src/tests/controls_tests.rs +++ b/crates/xlsx/src/tests/controls_tests.rs @@ -217,6 +217,68 @@ fn a_single_quoted_shape_id_is_dropped_from_the_kept_controls_too() { assert_eq!(back.sheet(0).unwrap().controls.len(), 1); } +/// One `mc:AlternateContent` holds a scroll bar, which is modelled, and a list box, which stays +/// with pass-through: only the scroll bar leaves the kept fragment, the list box keeps its +/// worksheet entry, its VML shape and its properties part through a save, and the file reads +/// back with the one modelled control and the kept one. +#[test] +fn an_unsupported_control_beside_a_modelled_one_in_one_alternate_content_is_kept() { + let rels = r#""#; + let prop_a = r#""#; + let prop_b = r#""#; + let vml = r##"2, 0, 3, 0, 5, 0, 4, 0$B$14001001102, 0, 6, 0, 5, 0, 9, 0$D$1:$D$32"##; + let anchor = |from: u32, to: u32| { + format!( + r#"20{from}050{to}0"# + ) + }; + let controls = format!( + r#"{}{}"#, + anchor(3, 4), + anchor(6, 9) + ); + let bytes = minimal( + &format!(r#"{controls}"#), + &[ + ("xl/worksheets/_rels/sheet1.xml.rels", rels), + ("xl/ctrlProps/a.xml", prop_a), + ("xl/ctrlProps/b.xml", prop_b), + ("xl/drawings/vmlDrawing1.vml", vml), + ], + "", + "", + ); + let (wb, _) = read_xlsx(&bytes).unwrap(); + let sh = wb.sheet(0).unwrap(); + assert_eq!(sh.controls.len(), 1, "the scroll bar is modelled, the list box is not"); + assert_eq!((sh.controls[0].kind, sh.controls[0].name.as_str()), (ControlKind::Slider, "Scroll 1")); + let kept = sh.preserved.as_ref().and_then(|k| k.elements.iter().find(|e| e.slot == "controls")).map(|e| e.xml.clone()).unwrap_or_default(); + assert!(kept.contains("List 1") && kept.contains(""), "the list box stays, in its wrapper: {kept}"); + assert!(!kept.contains("Scroll 1") && !kept.contains("shapeId=\"1025\""), "the scroll bar leaves every branch: {kept}"); + // Saved: the scroll bar once (one Fallback of ours), the list box in the kept wrapper with its + // shape and its properties part. + let out = write_xlsx(&wb).unwrap(); + let mut pkg = Package::open(&out).unwrap(); + let sheet = text(&mut pkg, "xl/worksheets/sheet1.xml"); + assert_eq!(sheet.matches(" = pkg.names().into_iter().filter(|n| n.starts_with("xl/ctrlprops/")).collect(); + assert_eq!(props.len(), 2, "ours and the kept one: {props:?}"); + assert!(props.iter().any(|n| text(&mut pkg, n).contains("objectType=\"List\"")), "the list box's properties part survives: {props:?}"); + // Read back: the one modelled control and the kept one. + let (back, _) = read_xlsx(&out).unwrap(); + let sh = back.sheet(0).unwrap(); + assert_eq!(sh.controls.len(), 1); + let kept = sh.preserved.as_ref().and_then(|k| k.elements.iter().find(|e| e.slot == "controls")).map(|e| e.xml.clone()).unwrap_or_default(); + assert!(kept.contains("List 1") && !kept.contains("Scroll 1"), "{kept}"); +} + /// Shapes with a named `id` and their number in `o:spid`, as some producers write them. const VML_SPID: &str = r##"
Go
1, 0, 6, 0, 3, 0, 7, 0[0]!Tidy