From d7f5a2d340bbf72e8ab63b89066095c85b55bb72 Mon Sep 17 00:00:00 2001 From: Rohith K Rangan Date: Thu, 6 Aug 2026 22:44:44 -0400 Subject: [PATCH 1/2] feat: preserve presentation slide identity --- src/formats/odf/mod.rs | 19 ++- src/formats/ppt/mod.rs | 49 +++++- src/formats/pptx/mod.rs | 41 +++-- src/package/relationships.rs | 4 - .../fixtures/odp/handmade-slide-identity.odp | Bin 0 -> 513 bytes .../fixtures/ppt/handmade-slide-identity.ppt | Bin 0 -> 9728 bytes .../pptx/handmade-slide-identity.pptx | Bin 0 -> 2908 bytes tests/gen_fixtures.py | 160 ++++++++++++++++++ tests/snapshots.rs | 61 +++++++ ...ots__odp__handmade-slide-identity.odp.snap | 13 ++ tests/snapshots/snapshots__odp__pres.odp.snap | 2 + ...pshots__ppt__handmade-multimaster.ppt.snap | 2 + ...ots__ppt__handmade-slide-identity.ppt.snap | 13 ++ ...pshots__ppt__handmade-sparsenotes.ppt.snap | 2 + tests/snapshots/snapshots__ppt__pres.ppt.snap | 2 + .../snapshots__pptx__handmade-links.pptx.snap | 2 + ...s__pptx__handmade-slide-identity.pptx.snap | 13 ++ .../snapshots/snapshots__pptx__pres.pptx.snap | 2 + 18 files changed, 347 insertions(+), 38 deletions(-) create mode 100644 tests/fixtures/odp/handmade-slide-identity.odp create mode 100644 tests/fixtures/ppt/handmade-slide-identity.ppt create mode 100644 tests/fixtures/pptx/handmade-slide-identity.pptx create mode 100644 tests/snapshots/snapshots__odp__handmade-slide-identity.odp.snap create mode 100644 tests/snapshots/snapshots__ppt__handmade-slide-identity.ppt.snap create mode 100644 tests/snapshots/snapshots__pptx__handmade-slide-identity.pptx.snap diff --git a/src/formats/odf/mod.rs b/src/formats/odf/mod.rs index c615feba..0bb7dd22 100644 --- a/src/formats/odf/mod.rs +++ b/src/formats/odf/mod.rs @@ -70,16 +70,27 @@ fn is_encrypted(pkg: &RefCell) -> Result { fn parse_presentation(pres: &Element, ctx: &Ctx) -> Result, ConvertError> { let mut blocks = Vec::new(); - for page in pres.find_all(ns::DRAW, "page") { + let mut emitted_content_slide = false; + for (slide_index, page) in pres.find_all(ns::DRAW, "page").enumerate() { + let mut page_blocks = + vec![Block::Paragraph(vec![Inline::Anchor(format!("slide-{}", slide_index + 1))])]; let mut title = Vec::new(); let mut body = Vec::new(); let mut notes = Vec::new(); walk_shapes(page, ctx, &mut title, &mut body, &mut notes)?; - blocks.append(&mut title); - blocks.append(&mut body); + page_blocks.append(&mut title); + page_blocks.append(&mut body); // Speaker notes are included (fixed policy), set off as a quote. if !notes.is_empty() { - blocks.push(Block::BlockQuote(notes)); + page_blocks.push(Block::BlockQuote(notes)); + } + let has_content = page_blocks.len() > 1; + if has_content && emitted_content_slide { + blocks.push(Block::Rule); + } + blocks.append(&mut page_blocks); + if has_content { + emitted_content_slide = true; } } Ok(blocks) diff --git a/src/formats/ppt/mod.rs b/src/formats/ppt/mod.rs index d0f38de2..5611d3f9 100644 --- a/src/formats/ppt/mod.rs +++ b/src/formats/ppt/mod.rs @@ -106,12 +106,20 @@ struct PendingShape { styles: Option, } +struct Segment { + blocks: Vec, + id: Option, + is_notes: bool, + slide_number: Option, +} + #[derive(Default)] struct Extractor { - /// Finished segments: (blocks, pairing id, is_notes). - segments: Vec<(Vec, Option, bool)>, + /// Finished slide and notes segments in source order. + segments: Vec, current: Vec, current_is_notes: bool, + current_slide_number: Option, list_run: Vec, pending: Option, /// Master style tables in master-list order: (masterId, styles). @@ -265,6 +273,7 @@ impl Extractor { ) -> Result<(), ConvertError> { // (persistIdRef, slideId) of the page whose container is pending. let mut pending: Option<(u32, u32)> = None; + let mut slide_number = 0usize; for (ver_inst, rec_type, body) in children(list) { match rec_type { // SlidePersistAtom: the next slide/notes page begins. @@ -273,6 +282,12 @@ impl Extractor { self.finish_slide(pending.take(), persist, data, container_type, is_notes)?; self.end_segment(id); self.current_is_notes = is_notes; + if is_notes { + self.current_slide_number = None; + } else { + slide_number += 1; + self.current_slide_number = Some(slide_number); + } pending = get_u32(body, 0).map(|p| (p, get_u32(body, 12).unwrap_or(0))); if !is_notes { self.select_master(pending.map(|(p, _)| p), persist, data); @@ -320,21 +335,26 @@ impl Extractor { fn end_segment(&mut self, id: Option) { self.flush_shape(); flush_list(&mut self.current, &mut self.list_run); - if !self.current.is_empty() { - let blocks = std::mem::take(&mut self.current); - self.segments.push((blocks, id, self.current_is_notes)); + let slide_number = self.current_slide_number.take(); + if !self.current.is_empty() || slide_number.is_some() { + self.segments.push(Segment { + blocks: std::mem::take(&mut self.current), + id, + is_notes: self.current_is_notes, + slide_number, + }); } } fn into_blocks(mut self) -> Vec { self.end_segment(None); - let mut slides: Vec<(Option, Vec)> = Vec::new(); + let mut slides: Vec<(Option, Option, Vec)> = Vec::new(); let mut notes: Vec<(Option, Vec)> = Vec::new(); - for (blocks, id, is_notes) in self.segments { + for Segment { blocks, id, is_notes, slide_number } in self.segments { if is_notes { notes.push((id, blocks)); } else { - slides.push((id, blocks)); + slides.push((slide_number, id, blocks)); } } // Notes pages pair to slides by their stored slide id, not by list @@ -342,7 +362,18 @@ impl Extractor { // slides), which order-based zipping would misattribute. let mut used = vec![false; notes.len()]; let mut out = Vec::new(); - for (sid, blocks) in slides { + let mut emitted_content_slide = false; + for (slide_number, sid, blocks) in slides { + if let Some(slide_number) = slide_number { + let has_content = !blocks.is_empty(); + if has_content && emitted_content_slide { + out.push(Block::Rule); + } + out.push(Block::Paragraph(vec![Inline::Anchor(format!("slide-{slide_number}"))])); + if has_content { + emitted_content_slide = true; + } + } out.extend(blocks); for (i, (nid, nblocks)) in notes.iter_mut().enumerate() { if !used[i] && sid.is_some() && *nid == sid { diff --git a/src/formats/pptx/mod.rs b/src/formats/pptx/mod.rs index 800c26ec..6450881f 100644 --- a/src/formats/pptx/mod.rs +++ b/src/formats/pptx/mod.rs @@ -31,8 +31,6 @@ const MASTER_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/slideMaster"; const NOTES_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/notesSlide"; -const SLIDE_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/slide"; - /// Namespaces whose markup this frontend understands; `mc:Choice` branches /// requiring anything else fall back to `mc:Fallback`. const SUPPORTED_NS: &[&str] = &[ns::P, ns::A, ns::R, ns::MC]; @@ -94,8 +92,7 @@ pub fn parse(bytes: &[u8]) -> Result { let mut failed = 0usize; let instance_counter = StdCell::new(0u64); // Every slide has a start anchor id so internal slide-to-slide links - // resolve after concatenation; the anchor node is emitted only on - // slides some link actually targets. + // resolve after concatenation. let slide_anchors: HashMap = slide_paths .iter() .enumerate() @@ -105,23 +102,21 @@ pub fn parse(bytes: &[u8]) -> Result { for p in &slide_paths { all_rels.push(read_rels(&mut pkg.borrow_mut(), &rels_part_for(p))?); } - let targeted: std::collections::HashSet = slide_paths - .iter() - .zip(&all_rels) - .flat_map(|(p, rels)| { - rels.iter() - .filter(|(_, r)| r.rel_type == SLIDE_REL && r.mode == TargetMode::Internal) - .filter_map(move |(_, r)| path::resolve(p, &r.target).ok().map(|t| t.path)) - }) - .filter(|t| slide_anchors.contains_key(t)) - .collect(); + let mut emitted_content_slide = false; for (slide_index, slide_path) in slide_paths.iter().enumerate() { + let anchor = slide_anchors + .get(slide_path) + .expect("every slide path has a corresponding anchor") + .clone(); + let mut slide_blocks = vec![Block::Paragraph(vec![Inline::Anchor(anchor)])]; + let tree = match pkg.borrow_mut().optional_xml_part(slide_path)? { Some(t) => t, None => { log::warn!("skipping unusable slide {slide_path}"); failed += 1; + blocks.extend(slide_blocks); continue; } }; @@ -132,6 +127,7 @@ pub fn parse(bytes: &[u8]) -> Result { else { log::warn!("skipping slide {slide_path}: no shape tree"); failed += 1; + blocks.extend(slide_blocks); continue; }; let slide_rels = &all_rels[slide_index]; @@ -163,12 +159,7 @@ pub fn parse(bytes: &[u8]) -> Result { instance_counter: &instance_counter, slide_anchors: &slide_anchors, }; - if targeted.contains(slide_path) - && let Some(anchor) = slide_anchors.get(slide_path) - { - blocks.push(Block::Paragraph(vec![Inline::Anchor(anchor.clone())])); - } - parse_shapes(sp_tree, &ctx, &mut blocks)?; + parse_shapes(sp_tree, &ctx, &mut slide_blocks)?; // Speaker notes, set off as a quote (fixed policy: included). The // tree is loaded before the `if let` so the package borrow is not @@ -203,9 +194,17 @@ pub fn parse(bytes: &[u8]) -> Result { } } if !notes_blocks.is_empty() { - blocks.push(Block::BlockQuote(notes_blocks)); + slide_blocks.push(Block::BlockQuote(notes_blocks)); } } + let has_content = slide_blocks.len() > 1; + if has_content && emitted_content_slide { + blocks.push(Block::Rule); + } + blocks.extend(slide_blocks); + if has_content { + emitted_content_slide = true; + } } if failed == slide_paths.len() { return Err(ConvertError::malformed("no slide in the presentation could be read")); diff --git a/src/package/relationships.rs b/src/package/relationships.rs index ecb74ff8..98b58f2c 100644 --- a/src/package/relationships.rs +++ b/src/package/relationships.rs @@ -48,10 +48,6 @@ impl Relationships { self.0.get(id).filter(|r| r.mode == TargetMode::Internal).map(|r| r.target.as_str()) } - pub fn iter(&self) -> impl Iterator { - self.0.iter().map(|(k, v)| (k.as_str(), v)) - } - /// The internal-mode relationship of a given type, lowest id first so /// the pick is deterministic when a producer emits duplicates. pub fn first_of_type(&self, rel_type: &str) -> Option<&Relationship> { diff --git a/tests/fixtures/odp/handmade-slide-identity.odp b/tests/fixtures/odp/handmade-slide-identity.odp new file mode 100644 index 0000000000000000000000000000000000000000..bc03b08623b6a998e63ff2c928c825d4a623854c GIT binary patch literal 513 zcmWIWW@Zs#fB?l9W3@Fa^nn}@<^bZ{%-qzH%7WCyf`XjPA%;Sw5s{eqjMrH;EZXlhUpH~9XQIVUo zHu#|55e0$0zeQWB*WTk{{I*S+0x)Esnf6%UEs2_qA_&e4l0glF(6W=vvyIo0{bpt9?r|Z|%g5 zt-JYNyg1t)(-fubxT)A|)nwJw>Dt`CCM9V-->kPpu_v+a{44#(+4Z~LdT+Do z5H0!J_(VOxn~_O`8F%Od!wL)<7(o;)E)cq~2QNeq149F2D3FO1*a6P{Aj}8S HHDF@_@>#TH literal 0 HcmV?d00001 diff --git a/tests/fixtures/ppt/handmade-slide-identity.ppt b/tests/fixtures/ppt/handmade-slide-identity.ppt new file mode 100644 index 0000000000000000000000000000000000000000..d98b781db3fca9f059ab2f4e072032375208af20 GIT binary patch literal 9728 zcmeI2y-EW?5Xa~4j2gc-0Z|DSSNaH65%Ht42tsO6lOmvrTuwBN_zJ#&Vwcu8u(Q-h zP;9JJ5bex4bGg+u3rTDQ<-cLGvoo_hv%jrw*qiI@?fFISjyhVNJZeR08Z`6{%Nadq zp%q1uO!b&$nN?_ihyc->8smGLF@gR82=oB50Js% AA^-pY literal 0 HcmV?d00001 diff --git a/tests/fixtures/pptx/handmade-slide-identity.pptx b/tests/fixtures/pptx/handmade-slide-identity.pptx new file mode 100644 index 0000000000000000000000000000000000000000..02356fb35d05d552fd4c393d01b7013fb829240a GIT binary patch literal 2908 zcmaKu2T+q)7ls3&3Wg|61StVFw2*`*B1J%I=m-eXA%hr9D3OR7`l_IjUQ|GeqSA{r zDT*`+RjGm?(sYHTNdQ;G{eYd}H|n36 zodE=D0)s%@z&~en33wtFPjopI|S8`3`T~Y?cI{d81)1>RFvvhb zo_rESGp9#SSV7|2m^%mVZp&Em;<^Mo!pN`O4uY+SJXH89HmF#o;fs}ZolWArkaGWB zLvB993)SwY$?ul%SEk#Ha3h~HU>&2AnlAi)bCr$%`;E+2t?G-+7C)G?>pJ_c1TcmK>~R23 z7k{iz08)PYm(o+e?*$49nizY?RyFzr0(<@B2Ak6Z%@c-2-lbo}_*)w48;x#6OnoDh z?5fO&& zPggzK^29q74x2FQ7^&VSObY{4@=P*~7JS{2~?U|0T z;=xQf%ccgH)hDWVe^f4Ze_=7q(>~|upk}Q5PyKLxg3~De{*cOZb4{oC;tyN!ER2NT zo%!orephHMG=vxtF5RPoPQ!b8L1v>qFP-7Q@er#7q5Vk7Bb|q*k2U1`+W7{wR`EXD z5`0#?)D@1cFe)d~xUSN9z@R45TY&vnFoHlL|9RLwWO^Grhh87Ay?y9<>lv`l@!Ucf zO#djXnpfk5Vea`7Q=LDe$HzrsGBQO^Hx)OJ640M#YH)AZ!2({)JoXUILjRFrhv&gn z!e(KXiMAbs_B_7Y;WO^2ckZ$tpDaAoOPNX~GNWGgsKN(a;nD%Z=SNP-6BTj~om+UI z_VRGh?GN0-83IczSV4rS50Sx#w7kLf)+Wia+W*sV7DWW_dydk|l$g~jHNm&o2g4{$ zxX4j6xFu?ntfdC(GPiz2k=Yigd zLVO3lNpvR)%?)B**?DrKaoI9sHgWO6AZ2dBsJ642_?2~LAv-dn>~`4g8rJIU@w7rW zaf{w@<;~!lNlwQPNrB`hmzdgGgKnKfGoz^^N}G&fgb_W84Jul)lq1P!11fH-6Nv}O zHoX?F?mr$>rjU!E>BMasTHu;5Ra}DFo@*<PFhqc z{evw`x)*hpvrfJ6_@RMW_+%W&xSi4y$@w5*_}7zPV1L7ZG%KX9!+IDnYw#Ph3cF?-0kfP} z7s>mY$q(k-?qo|O<$BkeCXHx)vcQ2qS_|3k-IH zbI0GrC83=j^G8pU!X3mb?{+1WSc%`GZ3)-I*+MN_S9{MupR!JNdyO^b&o%hCn?f0_ zGu?VMm&zPSsJt%Of5aIQ58hgw<_b2i87JSo?Cv1hq^j$kfOPDn1Qj^?$+0dQj3dQc z^tDX4ya@bjH>`(TwoEr*5R9U8*`q=%?Y6%(mHOetvF5c%O9OE9OfbS<+Cb(N|NIN+ zOkBP+zwV0PieX|%?-uAsnI3ysY{x`Aoo<#0bEqF z-K~_1vlT_}k`*~{)TR@$D>F#Zqi(o-X@qiY$b26G(^oK}g56Juz>*i1z&{#!s3wDv z0&KYbCSFRr@iN5$My@V?<856ZaN_cal=xGjvg6R7fm+XRq_<9RiCwn;y67d_H9VBX z-yT1{9)++Xn01#~dMGs>-3+XmH2EqoH5IczL_>Z2Z2(`ws`1Fgg{B_071P>BS<$wo zK5M1%`$eq6jjs>yA8D`V%p_;f7}8B@nsY3qv=sX7OPvcBA`@Ca-w-KU27h`)j+b@; zuC6nfolH>s27O9OvFB#0kUT5IdD6^$Zq2r0Cd&6j;p3S$c%;UB$!cwi-?h$1b*Sz- zVi3~Nk%eC%DfoWG23D*4po6#x_1QeFQ!~|*HpCj-i@6&1K+)6b&INbj=*4MO!9H)n z{u&@}$%@KYW=U7}$K+%*I(s0yUlOJ#I?D+qerrkewwX~qZ{92NI+)P{!@$S`{{6@V zByZ;v3JCw@07Va=pB#3VgFvASY5?MY<_P+Q^qqNcAvbW)zv|QU1p1z~M-T)Cz-_|+ z>2UNg`u4B~JHoUJ`>9#bBk1ek9)iUDGh(MI(j(|^{~n^5 + + + +""" + pres_rels = """ + + + + + +""" + + def slide(body): + return f""" + + + +{body} +""" + + def title(text): + return ( + '' + '' + f'{text}' + ) + + def body(text): + return ( + '' + '' + f'{text}' + ) + + slides = [ + slide(title("First title")), + slide(body("Untitled body")), + slide(""), + slide(title("Last title")), + ] + ct = """ + + + + + + + + +""" + root_rels = """ + + +""" + write_zip(OUT / "pptx" / "handmade-slide-identity.pptx", [ + ("[Content_Types].xml", ct), + ("_rels/.rels", root_rels), + ("ppt/presentation.xml", presentation), + ("ppt/_rels/presentation.xml.rels", pres_rels), + *[(f"ppt/slides/slide{i}.xml", value) for i, value in enumerate(slides, 1)], + ]) + + +# --------------------------------------------------------------------------- +# S21: every OpenDocument presentation page keeps its identity, including +# untitled and empty pages, without relying on an internal link targeting it. + +def slide_identity_odp(): + ns = ( + 'xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" ' + 'xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" ' + 'xmlns:presentation="urn:oasis:names:tc:opendocument:xmlns:presentation:1.0" ' + 'xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"' + ) + + def page(content): + return f'{content}' + + def frame(text, class_name=""): + cls = f' presentation:class="{class_name}"' if class_name else "" + return ( + f'{text}' + '' + ) + + content = f""" + + +{page(frame("First title", "title"))} +{page(frame("Untitled body"))} +{page("")} +{page(frame("Last title", "title"))} + +""" + write_zip( + OUT / "odp" / "handmade-slide-identity.odp", + [("content.xml", content)], + mimetype_first="application/vnd.oasis.opendocument.presentation", + ) + + # --------------------------------------------------------------------------- # S5: standard Word OLE markup - a VML preview image next to the # o:OLEObject; the object's identity and payload must win over the preview. @@ -1505,6 +1612,56 @@ def persist_atom(persist_ref, sid): ]) +# --------------------------------------------------------------------------- +# S21: every legacy presentation slide keeps its identity, including untitled +# and empty slides, without relying on an internal link targeting it. + +def slide_identity_ppt(): + def slide(text, tx_type): + slide_atom = ppt_rec(2, 0x03EF, struct.pack(" Vec { + doc.blocks + .iter() + .filter_map(|block| match block { + Block::Paragraph(inlines) => match inlines.as_slice() { + [Inline::Anchor(id)] if id.starts_with("slide-") => Some(id.clone()), + _ => None, + }, + _ => None, + }) + .collect() +} + +fn has_slide_link(doc: &anydoc::model::Document, target: &str) -> bool { + doc.blocks.iter().any(|block| match block { + Block::Paragraph(inlines) | Block::Heading { content: inlines, .. } => { + inlines.iter().any(|inline| { + matches!( + inline, + Inline::Link { target: LinkTarget::Anchor(id), .. } if id == target + ) + }) + } + _ => false, + }) +} + +#[test] +fn presentation_slides_are_anchored_and_separated() { + for (directory, filename, format) in [ + ("pptx", "handmade-slide-identity.pptx", anydoc::Format::Pptx), + ("ppt", "handmade-slide-identity.ppt", anydoc::Format::Ppt), + ("odp", "handmade-slide-identity.odp", anydoc::Format::Odp), + ] { + let path = fixture_root().join(directory).join(filename); + let bytes = std::fs::read(&path).unwrap(); + let doc = anydoc::to_document(&bytes, format).unwrap(); + assert_eq!( + slide_anchor_ids(&doc), + ["slide-1", "slide-2", "slide-3", "slide-4"], + "{filename}: slide anchors must follow source order" + ); + let rules = doc.blocks.iter().filter(|block| matches!(block, Block::Rule)).count(); + assert_eq!(rules, 2, "{filename}: empty slides must not double a rule"); + assert!(!matches!(doc.blocks.first(), Some(Block::Rule))); + assert!(!matches!(doc.blocks.last(), Some(Block::Rule))); + let markdown = anydoc::to_markdown(&path).unwrap(); + assert!(!markdown.contains("---\n\n---"), "{filename}: empty slides must not double rules"); + } +} + +#[test] +fn pptx_slide_links_still_target_the_sequential_anchor() { + let path = fixture_root().join("pptx").join("handmade-links.pptx"); + let bytes = std::fs::read(path).unwrap(); + let doc = anydoc::to_document(&bytes, anydoc::Format::Pptx).unwrap(); + assert_eq!(slide_anchor_ids(&doc), ["slide-1", "slide-2"]); + assert!(has_slide_link(&doc, "slide-2")); +} + /// Embedded object payloads land in `Document::assets` with their identity /// and media type (the Markdown output shows only the alt text). #[test] diff --git a/tests/snapshots/snapshots__odp__handmade-slide-identity.odp.snap b/tests/snapshots/snapshots__odp__handmade-slide-identity.odp.snap new file mode 100644 index 00000000..0fa59dc3 --- /dev/null +++ b/tests/snapshots/snapshots__odp__handmade-slide-identity.odp.snap @@ -0,0 +1,13 @@ +--- +source: tests/snapshots.rs +expression: output +--- +## First title + +--- + +Untitled body + +--- + +## Last title diff --git a/tests/snapshots/snapshots__odp__pres.odp.snap b/tests/snapshots/snapshots__odp__pres.odp.snap index d46d3477..4f3e359b 100644 --- a/tests/snapshots/snapshots__odp__pres.odp.snap +++ b/tests/snapshots/snapshots__odp__pres.odp.snap @@ -11,6 +11,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide | Region | Total | diff --git a/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap b/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap index e5e99b52..79f23522 100644 --- a/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap +++ b/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap @@ -4,4 +4,6 @@ expression: output --- - **Alpha master body text** +--- + *Beta master body text* diff --git a/tests/snapshots/snapshots__ppt__handmade-slide-identity.ppt.snap b/tests/snapshots/snapshots__ppt__handmade-slide-identity.ppt.snap new file mode 100644 index 00000000..0fa59dc3 --- /dev/null +++ b/tests/snapshots/snapshots__ppt__handmade-slide-identity.ppt.snap @@ -0,0 +1,13 @@ +--- +source: tests/snapshots.rs +expression: output +--- +## First title + +--- + +Untitled body + +--- + +## Last title diff --git a/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap b/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap index 812a4e5b..4b8b11ba 100644 --- a/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap +++ b/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap @@ -4,6 +4,8 @@ expression: output --- First slide text +--- + Second slide text > Notes for the second slide diff --git a/tests/snapshots/snapshots__ppt__pres.ppt.snap b/tests/snapshots/snapshots__ppt__pres.ppt.snap index 67a4c638..38a868da 100644 --- a/tests/snapshots/snapshots__ppt__pres.ppt.snap +++ b/tests/snapshots/snapshots__ppt__pres.ppt.snap @@ -12,6 +12,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide Region diff --git a/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap b/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap index aae0a015..b11df897 100644 --- a/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap +++ b/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap @@ -6,6 +6,8 @@ expression: output [External link](https://example.com/) +--- + Second slide content diff --git a/tests/snapshots/snapshots__pptx__handmade-slide-identity.pptx.snap b/tests/snapshots/snapshots__pptx__handmade-slide-identity.pptx.snap new file mode 100644 index 00000000..0fa59dc3 --- /dev/null +++ b/tests/snapshots/snapshots__pptx__handmade-slide-identity.pptx.snap @@ -0,0 +1,13 @@ +--- +source: tests/snapshots.rs +expression: output +--- +## First title + +--- + +Untitled body + +--- + +## Last title diff --git a/tests/snapshots/snapshots__pptx__pres.pptx.snap b/tests/snapshots/snapshots__pptx__pres.pptx.snap index 61f7f60c..a7b9a909 100644 --- a/tests/snapshots/snapshots__pptx__pres.pptx.snap +++ b/tests/snapshots/snapshots__pptx__pres.pptx.snap @@ -12,6 +12,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide | Region | Total | From bddfed5161e5e657584039abaef2fc97671d642d Mon Sep 17 00:00:00 2001 From: Vishhvak Srinivasan Date: Thu, 6 Aug 2026 23:00:22 -0400 Subject: [PATCH 2/2] fix: preserve positional anchors for duplicate pptx slides --- src/formats/pptx/mod.rs | 26 ++++---- .../pptx/handmade-duplicate-slide-target.pptx | Bin 0 -> 2457 bytes tests/gen_fixtures.py | 63 ++++++++++++++++++ tests/snapshots.rs | 16 +++++ ..._handmade-duplicate-slide-target.pptx.snap | 15 +++++ 5 files changed, 108 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/pptx/handmade-duplicate-slide-target.pptx create mode 100644 tests/snapshots/snapshots__pptx__handmade-duplicate-slide-target.pptx.snap diff --git a/src/formats/pptx/mod.rs b/src/formats/pptx/mod.rs index 6450881f..75601882 100644 --- a/src/formats/pptx/mod.rs +++ b/src/formats/pptx/mod.rs @@ -91,13 +91,16 @@ pub fn parse(bytes: &[u8]) -> Result { let mut blocks: Vec = Vec::new(); let mut failed = 0usize; let instance_counter = StdCell::new(0u64); - // Every slide has a start anchor id so internal slide-to-slide links - // resolve after concatenation. - let slide_anchors: HashMap = slide_paths - .iter() - .enumerate() - .map(|(i, p)| (p.clone(), format!("slide-{}", i + 1))) - .collect(); + // Slide identity is positional in `sldIdLst`, so this path-keyed map is + // only for resolving internal slide-to-slide links. If a malformed deck + // lists one part more than once, links keep the first occurrence's + // identity instead of being retargeted by a later map insertion. + let mut slide_anchors: HashMap = HashMap::new(); + for (slide_index, slide_path) in slide_paths.iter().enumerate() { + slide_anchors + .entry(slide_path.clone()) + .or_insert_with(|| format!("slide-{}", slide_index + 1)); + } let mut all_rels: Vec = Vec::with_capacity(slide_paths.len()); for p in &slide_paths { all_rels.push(read_rels(&mut pkg.borrow_mut(), &rels_part_for(p))?); @@ -105,11 +108,10 @@ pub fn parse(bytes: &[u8]) -> Result { let mut emitted_content_slide = false; for (slide_index, slide_path) in slide_paths.iter().enumerate() { - let anchor = slide_anchors - .get(slide_path) - .expect("every slide path has a corresponding anchor") - .clone(); - let mut slide_blocks = vec![Block::Paragraph(vec![Inline::Anchor(anchor)])]; + // Unlike link resolution above, emission must preserve every + // positional occurrence, including duplicate paths. + let mut slide_blocks = + vec![Block::Paragraph(vec![Inline::Anchor(format!("slide-{}", slide_index + 1))])]; let tree = match pkg.borrow_mut().optional_xml_part(slide_path)? { Some(t) => t, diff --git a/tests/fixtures/pptx/handmade-duplicate-slide-target.pptx b/tests/fixtures/pptx/handmade-duplicate-slide-target.pptx new file mode 100644 index 0000000000000000000000000000000000000000..20cd17e178f9122425ab604df3b185aac6bd64d2 GIT binary patch literal 2457 zcmaJ?2UHX38chf#M1-su2t|=9=$g=kRThvIN&rKVpeRTQN+b|!Aff~nELo+8-iZQA zk(Q-|r3fxX5wO4_C{5I?vJeCngKt33;gRRQnKSdxoH_qJbMJS*`{A$uXDs=AzJX3mA$4Fgk+M0(jZB~ z=*HWERhptflUyI&_Sj$1bAP*-F!x+AjLn$qPMD$2T*pFW&MjrsEm?$-|=&@ zsUmW|f7B`7#~w7|eyz}HSS@XzY(^|DuDDd0ymJ&h>##wM2qv>x z5{FRL4DI1`%YdJ)HXDM9RB{+K6!nuP7wShIOJ@^$bad@wTru*ZNGF)(L+k6ORS`z{ zy{T=Md!59KCQQn*bpNCf8fNE*Xo#MJ`*wvl3Oq%Fzqau-ZDEJq$M!WYdfzWo?$wsH z6)?5>)x0=6iXwefI2S$X5v_;bOpFvb&K>{=F#%$0?jVyx(d1x42w(-zP`|){H6vV) z?St%Ddctm>~Aemf3RcG~1( zvGs>&4Bv5sTX}0TP%!uY{@aGohnc^H$KcwewdF6rg0G|}BvwCUYFvx`{Tb|4wl`We zUWfXC15boynJyemOGgc=@Erag@2((G2G3ys8I50FqFvusXA&9C#s{Jx znP(LENHwM!}7-Ta?DT(C_Rsk3~a z9b$P&YCgYkvU@5o0jf|V6l}qOnBC6St4EMaSVKaZ*9M6r26O?8iYq?( z$ADjlNfwT!ft>dQ*v;-rdbZq0c5&jYmRD)p~E4(IFAc|55vZ~C7g zHWMep!>D>d-KhX^#CPH${(e4$5cK+@wO)KjyR{TxFXJygClO5&nN30n3%Ew@DlOfI zV#e)Msd)E?e#X0{e8WZUO+VJbpm!S9 z95M?qR&MFAAYB~-N z9j%ISlvd~vzQP#Mw2^0xNcY%ZmlUfEXoakp&8b2ELSm+)-^~>BUeCGfq{k{EnL^lRVCKlp)l!K}qFNl=s+HKPh_I54 zK|)%5WP8b#Bk2Pj!SW=+$FNL~%|gGq+;X@a5Lg}%d)H>6zZq>q?(dOevKA>qkJz`M zH7q|Ni()^0rO>?}K#CdlZMvcv>Pt()_&uoO=Z&~VPOSKmY6Fb+xM<0>ZODT&bwNcm zZ1l0Y7>6W#EN2JIH6fqm@W;tgd8cj0bA&MrXXW2D+6?ZOu3l6Usgwb>WX#iUKa~r+HBQ{q<;GjO|h=Iv)9xqjB1GKkx^@Co2ahe6xejKK5^l&z53{A za`8TqVNYp;yFA|ZXqy^iIXku1{A|)2UVWk99kSy{0aM{tT9c)X>9kAmyPsb4xbM75+_RtY52w6g6gUU>_X$ukXMZndE!AZ&qrL-;>2%MZpM zRvTb~g5SXYC%X6{_}g#;f+VyF;)hMikH+ux4K!EbO=#=?weEWyRtWe60*L_U7@+=5 Ik+rw~0I`w0+W-In literal 0 HcmV?d00001 diff --git a/tests/gen_fixtures.py b/tests/gen_fixtures.py index b8ea2614..c0f29e1e 100644 --- a/tests/gen_fixtures.py +++ b/tests/gen_fixtures.py @@ -1155,6 +1155,68 @@ def slide(body): ]) +# --------------------------------------------------------------------------- +# S22: duplicate slide parts in sldIdLst retain positional identities while +# internal links resolve to the first occurrence of their target part. + +def duplicate_slide_target_pptx(): + presentation = f""" + + + +""" + pres_rels = """ + + + + +""" + + def slide(body): + return f""" + + + +{body} +""" + + slide1 = slide( + '' + '' + 'Link to the first part' + '' + ) + slide1_rels = """ + + +""" + slide2 = slide( + '' + 'Middle slide' + ) + ct = """ + + + + + + +""" + root_rels = """ + + +""" + write_zip(OUT / "pptx" / "handmade-duplicate-slide-target.pptx", [ + ("[Content_Types].xml", ct), + ("_rels/.rels", root_rels), + ("ppt/presentation.xml", presentation), + ("ppt/_rels/presentation.xml.rels", pres_rels), + ("ppt/slides/slide1.xml", slide1), + ("ppt/slides/_rels/slide1.xml.rels", slide1_rels), + ("ppt/slides/slide2.xml", slide2), + ]) + + # --------------------------------------------------------------------------- # S21: every presentation slide keeps its identity, including untitled and # empty slides, without relying on an internal link targeting it. @@ -2067,6 +2129,7 @@ def main(): gaps_ods() encoded_docs() order_pptx() + duplicate_slide_target_pptx() slide_identity_pptx() slide_identity_odp() css_links_epub() diff --git a/tests/snapshots.rs b/tests/snapshots.rs index cee9171e..940a0f33 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -174,6 +174,22 @@ fn pptx_slide_links_still_target_the_sequential_anchor() { assert!(has_slide_link(&doc, "slide-2")); } +#[test] +fn pptx_duplicate_slide_targets_keep_positional_anchors() { + let path = fixture_root().join("pptx").join("handmade-duplicate-slide-target.pptx"); + let bytes = std::fs::read(path).unwrap(); + let doc = anydoc::to_document(&bytes, anydoc::Format::Pptx).unwrap(); + assert_eq!( + slide_anchor_ids(&doc), + ["slide-1", "slide-2", "slide-3"], + "duplicate sldIdLst targets must retain sequential, unique anchors" + ); + assert!( + has_slide_link(&doc, "slide-1"), + "a link to the duplicated part must resolve to its first occurrence" + ); +} + /// Embedded object payloads land in `Document::assets` with their identity /// and media type (the Markdown output shows only the alt text). #[test] diff --git a/tests/snapshots/snapshots__pptx__handmade-duplicate-slide-target.pptx.snap b/tests/snapshots/snapshots__pptx__handmade-duplicate-slide-target.pptx.snap new file mode 100644 index 00000000..000606c0 --- /dev/null +++ b/tests/snapshots/snapshots__pptx__handmade-duplicate-slide-target.pptx.snap @@ -0,0 +1,15 @@ +--- +source: tests/snapshots.rs +expression: output +--- + + +[Link to the first part](#slide-1) + +--- + +Middle slide + +--- + +[Link to the first part](#slide-1)