fix(pptx): round-trip preserve UnknownElement on save#39
Merged
Conversation
Slidewise's importer wraps OOXML it can't model — charts, SmartArt, group shapes, OLE, math, complex tables — into an UnknownElement carrying the raw XML (ooxmlXml). The serializer's case "unknown": return; meant every parse → save cycle silently destroyed those parts of the deck. - parsePptx now keeps the original archive bytes on the Deck and the source slide path on each Slide (non-enumerable so they don't pollute JSON dumps). - serializeDeck post-processes the zip pptxgenjs writes: for every slide whose Deck-side counterpart carries UnknownElement payloads, inject each ooxmlXml fragment into the generated <p:spTree>, copy the rels + media those fragments referenced from the source archive, renumber rIds to avoid clashes with what pptxgenjs already wrote, and uniquely prefix preserved media targets so they don't collide with media pptxgenjs allocated. - parsePptx also accepts Uint8Array directly so the server-side serialize → arrayBuffer → parsePptx loop works without an extra allocation. - New regression test exercises the full parse → serialize → re-parse loop on a synthetic SmartArt-style fragment, asserting that the fragment lands in the new spTree and the referenced PNG is copied to the output with a fresh rId. Verified end-to-end on the Dickinson sample (its bar chart on slide 4 is preserved as a graphicFrame → c:chart fragment plus chart1.xml copied across as ppt/charts/slidewise_preserved_0_chart1.xml).
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stops silent data loss on save when a deck contains charts, SmartArt, group shapes, OLE, math, or any other OOXML Slidewise couldn't model on import.
The bug
`deckToPptx.addElement`'s `case "unknown": return;` made the serializer ignore every `UnknownElement` — even though the importer was already preserving the raw OOXML on those elements (`UnknownElement.ooxmlXml`). Every parse → save cycle silently destroyed those parts of the deck.
The fix
Verified end-to-end
Round-tripped the Dickinson Sample deck: its bar chart on slide 4 (a `<p:graphicFrame>` wrapping `<c:chart r:id="rId2">`) survives — the graphicFrame XML is injected, `chart1.xml` is copied to `ppt/charts/slidewise_preserved_0_chart1.xml`, and the slide's rels now expose `rId3 → slidewise_preserved_0_chart1.xml`. Re-parsing the output yields the same 1 `UnknownElement` as the original.
What this doesn't try to do
Test plan