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..75601882 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]; @@ -93,35 +91,34 @@ 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; the anchor node is emitted only on - // slides some link actually targets. - 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))?); } - 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() { + // 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, None => { log::warn!("skipping unusable slide {slide_path}"); failed += 1; + blocks.extend(slide_blocks); continue; } }; @@ -132,6 +129,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 +161,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 +196,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 00000000..bc03b086 Binary files /dev/null and b/tests/fixtures/odp/handmade-slide-identity.odp differ diff --git a/tests/fixtures/ppt/handmade-slide-identity.ppt b/tests/fixtures/ppt/handmade-slide-identity.ppt new file mode 100644 index 00000000..d98b781d Binary files /dev/null and b/tests/fixtures/ppt/handmade-slide-identity.ppt differ 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 00000000..20cd17e1 Binary files /dev/null and b/tests/fixtures/pptx/handmade-duplicate-slide-target.pptx differ diff --git a/tests/fixtures/pptx/handmade-slide-identity.pptx b/tests/fixtures/pptx/handmade-slide-identity.pptx new file mode 100644 index 00000000..02356fb3 Binary files /dev/null and b/tests/fixtures/pptx/handmade-slide-identity.pptx differ diff --git a/tests/gen_fixtures.py b/tests/gen_fixtures.py index 21552bf7..c0f29e1e 100644 --- a/tests/gen_fixtures.py +++ b/tests/gen_fixtures.py @@ -1155,6 +1155,175 @@ 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. + +def slide_identity_pptx(): + presentation = f""" + + + +""" + 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 +1674,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")); +} + +#[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__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-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) 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 |