From d07639c7c4bd642e2af2e8737dcd70c2101dc0fa Mon Sep 17 00:00:00 2001
From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com>
Date: Sun, 25 Oct 2026 12:25:18 +0100
Subject: [PATCH] Warn for each skipped drawing object and chart on XLSX open

Fixes #2407

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 crates/xlsx/src/drawing.rs        | 16 ++++++++++-----
 crates/xlsx/src/tests/fixtures.rs | 34 +++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/crates/xlsx/src/drawing.rs b/crates/xlsx/src/drawing.rs
index 942cf4a53..0668ca9d8 100644
--- a/crates/xlsx/src/drawing.rs
+++ b/crates/xlsx/src/drawing.rs
@@ -124,8 +124,9 @@ fn mime_for(name: &str) -> &'static str {
 pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<(), IoError> {
     let Some(x) = cx.optional_xml(part)? else { return Ok(()) };
     let rels = cx.pkg.rels(part)?;
-    for a in &x.children {
+    for (n, a) in x.children.iter().enumerate() {
         let Some(anchor) = read_anchor(a, sheet) else { continue };
+        let n = n + 1;
         // Objects in markup-compatibility blocks: the first choice (chartex frames), else the
         // fallback.
         let objs: Vec<&El> = a
@@ -162,7 +163,10 @@ pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<(
                     let Some(gd) = obj.path(&["graphic", "graphicData"]) else { continue };
                     let uri = gd.attr("uri").unwrap_or("");
                     if let Some(rid) = gd.child("chart").and_then(|c| c.attr("id")) {
-                        let Some(rel) = rels.iter().find(|r| r.id == rid) else { continue };
+                        let Some(rel) = rels.iter().find(|r| r.id == rid) else {
+                            cx.warn(format!("a chart without its chart part was skipped ({part}, object {n})"));
+                            continue;
+                        };
                         let target = rel.target.clone();
                         if let Some(cx_xml) = cx.optional_xml(&target)? {
                             let chartex = uri.contains("chartex");
@@ -174,11 +178,13 @@ pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<(
                                 }
                                 None => cx.warn(format!("chart {target} has no supported plot and was skipped")),
                             }
+                        } else {
+                            cx.warn(format!("chart {target} is missing from the package and was skipped ({part}, object {n})"));
                         }
                     } else if uri.contains("chartex") || uri.contains("chartEx") {
-                        cx.warn("a histogram, waterfall, treemap… chart without its chart part was skipped");
+                        cx.warn(format!("a histogram, waterfall, treemap… chart without its chart part was skipped ({part}, object {n})"));
                     } else {
-                        cx.warn("an embedded object (SmartArt, OLE…) was skipped");
+                        cx.warn(format!("an embedded object (SmartArt, OLE…) was skipped ({part}, object {n})"));
                     }
                 }
                 "sp" | "cxnSp" => {
@@ -187,7 +193,7 @@ pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<(
                         sheet.shapes.push(Shape { id, ..s });
                     }
                 }
-                "grpSp" => cx.warn("grouped shapes are not supported and were skipped"),
+                "grpSp" => cx.warn(format!("grouped shapes are not supported and were skipped ({part}, object {n})")),
                 _ => {}
             }
         }
diff --git a/crates/xlsx/src/tests/fixtures.rs b/crates/xlsx/src/tests/fixtures.rs
index 0294f378f..08da55991 100644
--- a/crates/xlsx/src/tests/fixtures.rs
+++ b/crates/xlsx/src/tests/fixtures.rs
@@ -690,3 +690,37 @@ fn custom_widths_equal_to_the_default_round_trip() {
         sheet.child("cols").unwrap().kids("col").map(|c| (c.attr_u32("min"), c.attr_u32("max"), c.flag("customWidth", false))).collect();
     assert_eq!(cols, [(Some(4), Some(7), true)]);
 }
+
+#[test]
+fn every_skipped_drawing_object_is_reported() {
+    let frame = |row: u32, rid: &str| {
+        format!(
+            r#"<xdr:twoCellAnchor><xdr:from><xdr:col>1</xdr:col><xdr:colOff>0</xdr:colOff><xdr:row>{row}</xdr:row><xdr:rowOff>0</xdr:rowOff></xdr:from><xdr:to><xdr:col>3</xdr:col><xdr:colOff>0</xdr:colOff><xdr:row>{}</xdr:row><xdr:rowOff>0</xdr:rowOff></xdr:to><xdr:graphicFrame macro=""><xdr:nvGraphicFramePr><xdr:cNvPr id="2" name="C"/><xdr:cNvGraphicFramePr/></xdr:nvGraphicFramePr><xdr:xfrm><a:off x="0" y="0"/><a:ext cx="0" cy="0"/></xdr:xfrm><a:graphic><a:graphicData uri="http://schemas.microsoft.com/office/drawing/2014/chartex">{rid}</a:graphicData></a:graphic></xdr:graphicFrame><xdr:clientData/></xdr:twoCellAnchor>"#,
+            row + 5
+        )
+    };
+    let missing = r#"<c:chart xmlns:c="http://schemas.openxmlformats.org/drawingml/2006/chart" r:id="rId9"/>"#;
+    let gone = r#"<c:chart xmlns:c="http://schemas.openxmlformats.org/drawingml/2006/chart" r:id="rId1"/>"#;
+    let drawing = format!(
+        r#"<xdr:wsDr xmlns:xdr="http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing" xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships">{}{}{}{}{}</xdr:wsDr>"#,
+        frame(0, ""),
+        frame(10, ""),
+        frame(20, ""),
+        frame(30, missing),
+        frame(40, gone)
+    );
+    let drels = r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/chart" Target="../charts/chart9.xml"/></Relationships>"#;
+    let srels = r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing" Target="../drawings/drawing1.xml"/></Relationships>"#;
+    let bytes = super::minimal(
+        r#"<sheetData/><drawing r:id="rId1"/>"#,
+        &[("xl/worksheets/_rels/sheet1.xml.rels", srels), ("xl/drawings/drawing1.xml", &drawing), ("xl/drawings/_rels/drawing1.xml.rels", drels)],
+        "",
+        "",
+    );
+    let (_, rep) = read_xlsx(&bytes).unwrap();
+    for n in 1..=5 {
+        let tag = format!("object {n})");
+        let hits = rep.warnings.iter().filter(|w| w.contains("skipped") && w.contains("drawing1.xml") && w.contains(&tag)).count();
+        assert_eq!(hits, 1, "object {n}: {:?}", rep.warnings);
+    }
+}