From 706127402a58a20b2e5f8de09a99afdeb786d0bc Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:41:59 +0200 Subject: [PATCH] XLSX: keep header and footer pictures on save (part of #1257) --- crates/xlsx/src/passthrough.rs | 71 +++++++++++++--- crates/xlsx/src/sheet_write.rs | 2 +- crates/xlsx/src/tests/passthrough_tests.rs | 94 ++++++++++++++++++++++ crates/xlsx/src/write.rs | 1 + docs/file-format-parity.md | 5 +- 5 files changed, 160 insertions(+), 13 deletions(-) diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index dd881c1e9..03bde2bb5 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -6,7 +6,8 @@ //! their ``, data connections, XML maps) and each sheet (query tables, cells //! mapped to XML elements, form controls, ActiveX controls, OLE objects, the sheet background and //! printer settings, with the sheet's ``, `` and `` XML, the `r:id` -//! of `` and the legacy VML shapes those controls need). The workbook also keeps its +//! of ``, the legacy VML shapes those controls need and the pictures of the headers and +//! footers, `&G`, with their ``). The workbook also keeps its //! `` and the calcFeatures extension, which Excel needs to trust the saved values. //! //! On write, kept parts get fresh part names where needed and new relationship ids; the `r:id`s in @@ -92,7 +93,7 @@ const FILE_VERSION_ATTRS: &[&str] = &["appName", "lastEdited", "lowestEdited", " const CALC_FEATURES_URI: &str = "{B58B0392-4F1F-4190-BB64-5DF3571DCE5F}"; const NS_CALC_FEATURES: &str = "http://schemas.microsoft.com/office/spreadsheetml/2018/calcfeatures"; /// Kept sheet elements, in schema order (all after ``, before ``). -const SHEET_SLOTS: &[&str] = &["picture", "oleObjects", "controls"]; +const SHEET_SLOTS: &[&str] = &["legacyDrawingHF", "picture", "oleObjects", "controls"]; /// Relationship kinds GridCraft reads and rewrites, or that depend on content it rewrites: /// kept content whose graph reaches one of them is dropped. @@ -479,7 +480,10 @@ impl KeptReader { cx.warn(format!("sheet \"{name}\": {} was dropped: {why}", kind(&r.rel_type))); } } - if rels.iter().any(|r| is_kind(r, SHEET_ELEMENT_KINDS)) { + // Notes have one VML drawing per sheet; another one holds the header and footer pictures. + let vml_drawings = rels.iter().filter(|r| is_kind(r, &["vmlDrawing"])).count(); + let notes = rels.iter().filter(|r| is_kind(r, &["comments"])).count(); + if rels.iter().any(|r| is_kind(r, SHEET_ELEMENT_KINDS)) || vml_drawings > notes { self.sheet_elements(cx, part, name, &rels, &mut p, &mut seen); } (!p.is_empty()).then(|| Arc::new(p)) @@ -503,6 +507,21 @@ impl KeptReader { Err(why) => cx.warn(format!("sheet \"{name}\": printer settings were dropped: {why}")), } } + // Header and footer pictures (`&G`): a VML drawing of their own and the images it shows, + // kept byte for byte (GridCraft writes the header text, not the pictures). + if let Some(id) = scan.legacy_drawing_hf.as_deref() + && scan.legacy_drawing.as_deref() != Some(id) + && let Some(r) = rels.iter().find(|r| r.id == id && is_kind(r, &["vmlDrawing"])) + { + match self.keep_through(cx, p, std::slice::from_ref(r), seen, &["vmlDrawing"]) { + Ok(()) => { + let xml = format!("", esc_attr(&r.id)); + p.elements.push(PreservedXml { slot: "legacyDrawingHF".into(), xml }); + cx.warn(format!("sheet \"{name}\": pictures in the header and footer are not shown; they are kept for saving")); + } + Err(why) => cx.warn(format!("sheet \"{name}\": header and footer pictures were dropped: {why}")), + } + } let mut frags = scan.fragments; // Controls and OLE objects are drawn from shapes in the legacy VML drawing. if frags.iter().any(|(slot, _)| slot != "picture") { @@ -581,18 +600,20 @@ struct SheetScan { /// (slot, XML with the namespace declarations it needs). fragments: Vec<(String, String)>, legacy_drawing: Option, + /// The relationship id on `` (header and footer pictures). + legacy_drawing_hf: Option, /// The relationship id on `` (printer settings). page_setup: Option, } /// Finds the sheet's ``, ``, `` (also inside -/// ``) and the `` id. `None` when the XML is damaged. +/// ``) and the `` and `` ids. `None` when the XML is damaged. fn scan_sheet(bytes: &[u8]) -> Option { let data = xml::to_utf8(bytes); let text = std::str::from_utf8(&data).ok()?; let mut reader = Reader::from_str(text); let mut decls: Vec<(String, String)> = vec![]; - let mut out = SheetScan { fragments: vec![], legacy_drawing: None, page_setup: None }; + let mut out = SheetScan { fragments: vec![], legacy_drawing: None, legacy_drawing_hf: None, page_setup: None }; let mut in_root = false; loop { let start = usize::try_from(reader.buffer_position()).ok()?; @@ -614,6 +635,7 @@ fn scan_sheet(bytes: &[u8]) -> Option { match local.as_slice() { b"pageSetup" => out.page_setup = rel_id_attr(&e), b"legacyDrawing" => out.legacy_drawing = rel_id_attr(&e), + b"legacyDrawingHF" => out.legacy_drawing_hf = rel_id_attr(&e), _ => {} } reader.read_to_end(e.name()).ok()?; @@ -626,12 +648,12 @@ fn scan_sheet(bytes: &[u8]) -> Option { } Event::Empty(e) if in_root => { let local = e.local_name().as_ref().to_vec(); - if local == b"legacyDrawing" || local == b"pageSetup" { + if local == b"legacyDrawing" || local == b"legacyDrawingHF" || local == b"pageSetup" { let id = rel_id_attr(&e); - if local == b"pageSetup" { - out.page_setup = id; - } else { - out.legacy_drawing = id; + match local.as_slice() { + b"pageSetup" => out.page_setup = id, + b"legacyDrawing" => out.legacy_drawing = id, + _ => out.legacy_drawing_hf = id, } } else if local == b"picture" && let Some(frag) = text.get(start..usize::try_from(reader.buffer_position()).ok()?) @@ -738,6 +760,19 @@ fn shape_numbers(vml: &str) -> Vec { .collect() } +/// The highest shape block VML text claims in `` or uses in a shape id. +fn vml_block_max(vml: &str) -> u64 { + let used = shape_numbers(vml).into_iter().max().unwrap_or(0) / 1024; + let claimed = vml + .find("().ok()).max().unwrap_or(0)) + .unwrap_or(0); + used.max(claimed) +} + /// Kept VML (controls, OLE objects) plus GridCraft's note shapes: the notes get ids in a fresh /// shape block so they can't collide with the kept shapes the sheet XML refers to. fn merge_vml(kept: &str, ours: &str) -> String { @@ -837,11 +872,27 @@ fn map_start(e: &BytesStart<'_>, prefixes: &mut HashSet>, map: &mut dyn pub struct KeptOut { used: HashSet, n: u32, + /// The highest VML shape block a kept header and footer drawing uses: notes go after it. + vml_blocks: u32, /// Package-root relationships (type, target, external), added when `_rels/.rels` is written. root: Vec<(String, String, bool)>, } impl KeptOut { + /// Reserves the VML shape blocks the kept header and footer drawings of `wb` use, so that the + /// notes drawings GridCraft writes don't reuse their shape ids. + pub fn reserve_vml_blocks(&mut self, wb: &Workbook) { + let vmls = + wb.sheets.iter().filter_map(|s| s.preserved.as_deref()).flat_map(|p| &p.parts).filter(|q| q.content_type.eq_ignore_ascii_case(CT_VML)); + let max = vmls.map(|q| vml_block_max(&String::from_utf8_lossy(&q.data))).max().unwrap_or(0); + self.vml_blocks = u32::try_from(max).unwrap_or(u32::MAX); + } + + /// The shape block of the `n`th notes drawing. + pub fn notes_block(&self, n: u32) -> u32 { + n.saturating_add(self.vml_blocks) + } + /// A part name for a kept part: its original name when that can't collide with anything /// written, otherwise a fresh one in the same folder. fn claim(&mut self, original: &str) -> String { diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..2d6836b6d 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -551,7 +551,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( } out.default_type("vml", "application/vnd.openxmlformats-officedocument.vmlDrawing"); let vml_name = format!("xl/drawings/vmlDrawing{n}.vml"); - let vml = kept.notes_vml(vml_xml(sheet, n), &vml_name, out); + let vml = kept.notes_vml(vml_xml(sheet, out.kept.notes_block(n)), &vml_name, out); out.part(&vml_name, None, vml.into_bytes()); rels.add("comments", &format!("../comments{n}.xml")); let rid = rels.add("vmlDrawing", &format!("../drawings/vmlDrawing{n}.vml")); diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index d844daa05..23a7cdea5 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -823,3 +823,97 @@ fn file_version_and_calc_features_survive_a_save() { let plain = Unpacked::new(&write_xlsx(&gridcraft_model::Workbook::new()).unwrap()).text("xl/workbook.xml"); assert!(!plain.contains("fileVersion") && !plain.contains("extLst"), "{plain}"); } + +const LOGO: &[u8] = b"\x89PNG\r\n\x1a\n letterhead logo"; +const SIGNATURE: &[u8] = b"\x89PNG\r\n\x1a\n footer signature"; +const HF_VML: &str = r##""##; + +/// #1257: a letterhead in the header and a picture in the footer (`&G`), drawn from a VML part +/// that `` names, next to a note whose VML drawing GridCraft rewrites. +fn header_picture_package() -> Vec { + let types = r#""#; + let workbook = format!( + r#""# + ); + let sheet = format!( + r#"1&L&G&CInvoice&C&G"# + ); + let root_rels = rels(&[("rId1", "officeDocument", "xl/workbook.xml")]); + let wb_rels = rels(&[("rId1", "worksheet", "worksheets/sheet1.xml")]); + let sheet_rels = rels(&[ + ("rId1", "comments", "../comments1.xml"), + ("rId2", "vmlDrawing", "../drawings/vmlDrawing1.vml"), + ("rId3", "vmlDrawing", "../drawings/vmlDrawing2.vml"), + ]); + let hf_rels = rels(&[("rId1", "image", "../media/image1.png"), ("rId2", "image", "../media/image2.png")]); + make_zip(&[ + ("[Content_Types].xml", types.as_bytes()), + ("_rels/.rels", root_rels.as_bytes()), + ("xl/workbook.xml", workbook.as_bytes()), + ("xl/_rels/workbook.xml.rels", wb_rels.as_bytes()), + ("xl/worksheets/sheet1.xml", sheet.as_bytes()), + ("xl/worksheets/_rels/sheet1.xml.rels", sheet_rels.as_bytes()), + ("xl/comments1.xml", COMMENTS.as_bytes()), + ("xl/drawings/vmlDrawing1.vml", VML.as_bytes()), + ("xl/drawings/vmlDrawing2.vml", HF_VML.as_bytes()), + ("xl/drawings/_rels/vmlDrawing2.vml.rels", hf_rels.as_bytes()), + ("xl/media/image1.png", LOGO), + ("xl/media/image2.png", SIGNATURE), + ]) +} + +#[test] +fn header_and_footer_pictures_survive_a_save() { + let (wb, rep) = read_xlsx(&header_picture_package()).unwrap(); + assert!(!rep.warnings.iter().any(|w| w.contains("dropped")), "{:?}", rep.warnings); + assert_eq!(wb.sheet(0).unwrap().comments.len(), 1, "the note is modelled"); + // Twice: what was kept on the first save is read back and kept again. + let (again, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + for saved in [write_xlsx(&wb).unwrap(), write_xlsx(&again).unwrap()] { + let u = Unpacked::new(&saved); + u.assert_valid(); + let sheet = sheet_part(&u, "Invoice"); + let sx = u.text(&sheet); + assert!(sx.contains("&L&G&CInvoice"), "{sx}"); + let (ty, vml) = u.target(&sheet, &attr_of(&sx, "legacyDrawingHF", "r:id")); + assert!(ty.ends_with("/vmlDrawing"), "{ty}"); + assert_eq!(u.text(&vml), HF_VML); + let pictures: Vec<(String, Vec)> = u.rels(&vml).into_iter().map(|r| (r.0, u.parts.get(&r.2).unwrap().clone())).collect(); + assert_eq!(pictures, [("rId1".to_string(), LOGO.to_vec()), ("rId2".to_string(), SIGNATURE.to_vec())]); + // The notes keep their own drawing; CT_Worksheet puts legacyDrawingHF right after it. + let notes = u.target(&sheet, &attr_of(&sx, "legacyDrawing", "r:id")).1; + assert_ne!(notes, vml); + assert!(u.text(¬es).contains("ObjectType=\"Note\"")); + assert!(sx.find("`) and the blocks of its shape ids +/// (`_x0000_s1025` is in block 1). +fn vml_blocks(vml: &str) -> (Vec, Vec) { + let i = vml.find("().parse::().unwrap() / 1024) + .collect(); + (idmap, spids) +} + +#[test] +fn notes_take_shape_blocks_the_header_pictures_dont_use() { + let (wb, _) = read_xlsx(&header_picture_package()).unwrap(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + let sheet = sheet_part(&u, "Invoice"); + let sx = u.text(&sheet); + let (hf_map, hf_ids) = vml_blocks(&u.text(&u.target(&sheet, &attr_of(&sx, "legacyDrawingHF", "r:id")).1)); + let (notes_map, notes_ids) = vml_blocks(&u.text(&u.target(&sheet, &attr_of(&sx, "legacyDrawing", "r:id")).1)); + assert_eq!((hf_map.as_slice(), hf_ids.as_slice()), ([1].as_slice(), [1, 1].as_slice()), "the header drawing is kept as it was"); + assert!(!notes_ids.is_empty()); + for b in notes_map.iter().chain(¬es_ids) { + assert!(!hf_map.contains(b) && !hf_ids.contains(b), "notes reuse shape block {b}: {notes_map:?} {notes_ids:?}"); + } + assert!(notes_ids.iter().all(|b| notes_map.contains(b)), "note ids outside their drawing's blocks"); +} diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 06761f01d..c7e21e30e 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -212,6 +212,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { cell_pictures: Default::default(), kept: Default::default(), }; + out.kept.reserve_vml_blocks(wb); out.default_type("rels", "application/vnd.openxmlformats-package.relationships+xml"); out.default_type("xml", "application/xml"); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 158149fb3..2c95c870e 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** trivial (pictures in XLSX headers and footers are kept on save (#1257); previously trivial: formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -22,7 +22,7 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft read | GridCraft write | Fidelity / notes | Tests | |---|---|---|---|---|---| -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | +| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), pictures in headers and footers (`&G`, #1257: their VML drawing and images are written back unchanged; they are not shown, and can't be inserted), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, notes (with their shown/hidden state) and threaded comments with replies, resolved state and timestamps (`threadedComments` and `persons` parts, #300), hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, Allow Edit Ranges (`protectedRanges` with their password hashes, #307), calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Strict Open XML | R/W | yes | no (writes transitional) | Excel also saves transitional by default | fixtures | @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | trivial | XLSX: pictures in headers and footers (`&G`, the VML drawing `` names and its images) are kept on save; they are still not shown (#1257) | | 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) |