From 7ac304d0585c53518107b46403a2aff3b10a0d5f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 11:12:59 +0200 Subject: [PATCH 01/13] feat(keynote): open a `.key` deck and read the text of its slides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.key` package now opens as a presentation rather than as the zip it is made of, and each slide comes out as a page of text boxes placed where the file puts them. Which app wrote a package no longer comes off the root archive alone. Pages numbers its archives from 10000, but `KN.DocumentArchive` and `TN.DocumentArchive` are *both* type 1 — the id space is namespaced per app — so a `.key` and a `.numbers` are indistinguishable at the root. A Keynote package is the one that holds `Slide` components, which costs `Index/Metadata.iwa` and is only read once the root archive has come back as type 1. A slide names its title and body placeholders beside its drawable list, and the drawable list is the one to walk: a placeholder the slide leaves empty is named but not drawn. Each drawable resolves to the same `TSWP.StorageArchive` the Pages body already reads, so paragraphs and line breaks come for free — the paragraph mark is `\r` here where Pages writes `\n`, which the run table makes a detail either way. Frames arrived here rather than in the planned stage 4: a slide is drawables on a canvas, so there was no text-and-nothing-else shape to land first. A size of zero is a box that grows with its text, not a box of zero height, and is reported as no measure at all. Masters, styles, images, tables and presenter notes are still to come. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CHKtXe9U6EUgikLhNQq7Sm --- AGENTS.md | 11 +- CHANGELOG.md | 5 +- README.md | 3 +- src/odr/internal/file_type_table.cpp | 9 +- src/odr/internal/iwork/AGENTS.md | 81 +++++- src/odr/internal/iwork/PLAN.md | 62 ++-- src/odr/internal/iwork/iwork_archive.cpp | 5 + src/odr/internal/iwork/iwork_archive.hpp | 4 + src/odr/internal/iwork/iwork_document.cpp | 165 ++++++++--- src/odr/internal/iwork/iwork_document.hpp | 8 +- .../internal/iwork/iwork_element_registry.cpp | 56 ++++ .../internal/iwork/iwork_element_registry.hpp | 35 +++ src/odr/internal/iwork/iwork_file.cpp | 46 ++- src/odr/internal/iwork/iwork_parser.cpp | 275 +++++++++++++++--- src/odr/internal/iwork/iwork_parser.hpp | 6 + src/odr/internal/iwork/iwork_protobuf.cpp | 9 + src/odr/internal/iwork/iwork_protobuf.hpp | 2 + src/odr/internal/iwork/iwork_types.hpp | 87 +++++- src/odr/internal/open_strategy.cpp | 2 +- test/CMakeLists.txt | 1 + test/data.cmake | 4 +- test/src/internal/iwork/iwork_test_util.hpp | 130 +++++++++ test/src/internal/iwork/keynote_test.cpp | 234 +++++++++++++++ test/src/internal/iwork/pages_test.cpp | 5 +- 24 files changed, 1107 insertions(+), 138 deletions(-) create mode 100644 test/src/internal/iwork/keynote_test.cpp diff --git a/AGENTS.md b/AGENTS.md index b2f22a43..e98ffcb4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,7 +61,7 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme | `src/odr/internal/odf/` | OpenDocument (odt/ods/odp/odg); see [`odf/AGENTS.md`](src/odr/internal/odf/AGENTS.md). | | `src/odr/internal/ooxml/` | OOXML (docx/pptx/xlsx); see [`ooxml/AGENTS.md`](src/odr/internal/ooxml/AGENTS.md) + per-format docs. | | `src/odr/internal/oldms/` | **Legacy MS binary** (.doc/.ppt/.xls). | -| `src/odr/internal/iwork/` | Apple iWork (`.pages` today); see [`iwork/AGENTS.md`](src/odr/internal/iwork/AGENTS.md) + [`iwork/PLAN.md`](src/odr/internal/iwork/PLAN.md). | +| `src/odr/internal/iwork/` | Apple iWork (`.pages` and `.key` today); see [`iwork/AGENTS.md`](src/odr/internal/iwork/AGENTS.md) + [`iwork/PLAN.md`](src/odr/internal/iwork/PLAN.md). | | `src/odr/internal/pdf/` | PDF (own parser). | | `src/odr/internal/rtf/` | RTF, read as a text document; see [`rtf/AGENTS.md`](src/odr/internal/rtf/AGENTS.md) + [`rtf/PLAN.md`](src/odr/internal/rtf/PLAN.md). | | `src/odr/internal/markdown/` | Markdown (CommonMark + GFM via md4c), decoded to a text document; see [`markdown/AGENTS.md`](src/odr/internal/markdown/AGENTS.md) + [`markdown/PLAN.md`](src/odr/internal/markdown/PLAN.md). | @@ -228,10 +228,11 @@ Dispatch `release.yml` against main, publish the draft that appears — ## Apple iWork (`iwork`) -`.pages` opens as a text document and renders its body text; `.numbers` and -`.key` are named but not decoded. There is no spec — the module cites fixtures -instead, keeps its own Snappy and protobuf readers, and fails soft on archive -types it has not mapped. Read [`iwork/AGENTS.md`](src/odr/internal/iwork/AGENTS.md) +`.pages` opens as a text document and `.key` as a presentation; `.numbers` is +named but not decoded. There is no spec — the module cites fixtures instead, +keeps its own Snappy and protobuf readers, and fails soft on archive types it +has not mapped. Archive type ids are namespaced per app, so a `.key` and a +`.numbers` share them and the component list is what tells the two apart. Read [`iwork/AGENTS.md`](src/odr/internal/iwork/AGENTS.md) before touching it, and [`iwork/PLAN.md`](src/odr/internal/iwork/PLAN.md) for what comes next. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c49bf3c..95b40b08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,10 @@ The release run heads these entries with the version and opens a fresh that is one file rather than a package. - A `.pages` file opens as a text document and renders its body text instead of the zip it is made of; styles, images and tables are not read yet. `.numbers` - and `.key` are named but have no decoder. + is named but has no decoder. +- A `.key` file opens as a presentation and renders the text of each slide in + boxes where the file positions them, instead of the zip it is made of; slide + masters, styles, images, tables and presenter notes are not read yet. - An rtf opens and renders as a text document instead of throwing `UnknownFileType`. Its text, encoding, paragraphs and tabs are read; formatting, tables and pictures are not yet. diff --git a/README.md b/README.md index 8c59bcc9..a7ad8dff 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ C++ library to visualize files, especially documents, in HTML. - [pdf](https://github.com/opendocument-app/OpenDocument.core/issues/108) - pages (Apple Pages — body text only; styles, page geometry, images and tables are not read yet) +- key (Apple Keynote — the text of each slide, in boxes where the file puts + them; styles, masters, images, tables and presenter notes are not read yet) - rtf (body text only; character and paragraph formatting, tables and pictures are not read yet) - txt @@ -39,7 +41,6 @@ opening one throws: - xlsb (Excel binary workbook — an OOXML package whose workbook parts are binary rather than spreadsheetml) - numbers (Apple Numbers) -- key (Apple Keynote) ## Asking what is supported diff --git a/src/odr/internal/file_type_table.cpp b/src/odr/internal/file_type_table.cpp index 211702e7..8a421c64 100644 --- a/src/odr/internal/file_type_table.cpp +++ b/src/odr/internal/file_type_table.cpp @@ -790,8 +790,8 @@ constexpr std::array table{ .open = true, .translate_html = true, .color_scheme = true}}, - // Classified so a caller can name these two and hand their MIME types to a - // file picker; no engine reads either yet. + // Classified so a caller can name it and hand its MIME types to a file + // picker; no engine reads one yet. Row{FileType::iwork_numbers, "numbers"sv, numbers_extensions, @@ -805,7 +805,10 @@ constexpr std::array table{ keynote_mimetypes, FileCategory::document, DocumentType::presentation, - {}}, + {.detect_by_content = true, + .open = true, + .translate_html = true, + .color_scheme = true}}, }; /// Finds the row whose list, selected by @p list, contains @p needle. diff --git a/src/odr/internal/iwork/AGENTS.md b/src/odr/internal/iwork/AGENTS.md index 6b0b9b68..9e945fd6 100644 --- a/src/odr/internal/iwork/AGENTS.md +++ b/src/odr/internal/iwork/AGENTS.md @@ -4,9 +4,11 @@ Read the root [`AGENTS.md`](../../../../AGENTS.md) first, then [`PLAN.md`](PLAN.md), which is where this module is going and in what order. This file is what the landed stages decided, and why. -Landed: **stage 1** (detection and the container) and **stage 2** (Pages body -text). A `.pages` opens as a text document and renders its paragraphs. -Everything else in `PLAN.md` is still ahead. +Landed: **stage 1** (detection and the container), **stage 2** (Pages body +text) and **stage 5** (Keynote slides). A `.pages` opens as a text document and +renders its paragraphs; a `.key` opens as a presentation and renders each +slide's text boxes as frames where the geometry puts them. Everything else in +`PLAN.md` is still ahead. ## There is no spec, so a fixture is the citation @@ -79,17 +81,59 @@ The one place that skips the component list is detection: `IworkFile` reads `/Index/Document.iwa` directly, because it runs on every zip a caller opens and `Document` is the component whose file name never carries a suffix. -## Which app wrote the package comes off the root archive +## Which app wrote the package — the root archive, then the components `TP.DocumentArchive` is type 10000, verified on both `.pages` fixtures. The extension is not consulted — a caller may have lost it — and neither is `Metadata/Properties.plist`, which names an app version but not the app. -That is also why only `.pages` is detected. `iwork_numbers` and `iwork_keynote` -have `file_type_table.cpp` rows so a caller can name them and hand a file -picker their MIME types, but no capabilities: reading their root archive types -off a guess is exactly what this module does not do, and neither has a fixture -in the test data yet. +**Type ids are namespaced per app, and Keynote and Numbers collide.** Pages +numbers its archives from 10000, but `KN.DocumentArchive` and +`TN.DocumentArchive` are *both* type 1: `empty.key` and `empty.numbers` +`Index/Document.iwa` object 1 (iWork 14.4). So is the archive below each — +`KN.ShowArchive` and `TN.SheetArchive` are both type 2. The root archive alone +cannot tell the two apart, and a table mapping id → archive is only meaningful +once you already know the app. Only the `TS*` frameworks — `TSWP` text storage, +`TSD` drawables, `TST` tables — mean the same thing in all three. + +What does tell them apart is the **component list**: Keynote writes one `Slide` +component per slide and neither Numbers nor Pages writes any. Checked against +all six fixtures. That costs `Index/Metadata.iwa`, which detection otherwise +avoids reading, so it is only reached once the root archive has already come +back as type 1. + +`iwork_numbers` still has a `file_type_table.cpp` row with no capabilities so a +caller can name it and hand a file picker its MIME types; nothing decodes one +yet. + +## A slide is drawables, and the drawable list is the one to walk + +`KN.DocumentArchive` → `KN.ShowArchive` (field 2) → the slide tree (field 3), +whose repeated field 2 names one `KN.SlideNodeArchive` per slide in +presentation order; a node's field 2 is the `KN.SlideArchive` itself. + +A slide names its title and body placeholders in fields 5 and 6 *and* repeats +its drawables in field 7. Read **field 7**: a placeholder the slide leaves +empty is named in field 5/6 but is not in the drawable list, so walking the +list renders what is on the slide and nothing else — `style-various-1.key` +slide 3 has an empty body and slide 4 empty title and body. (Field 42 repeats +the same list on all four slides; nothing needs both.) + +A drawable is a `TSWP.ShapeArchive` (2011) or a `KN.PlaceholderArchive` (7), +which is the same shape one level deeper — its field 1. Either way field 2 of +the shape references the `TSWP.StorageArchive` that stage 2 already reads. +Every other drawable kind is skipped: `style-various-1.key` slide 4 carries a +`TST.TableInfoArchive` (6000) that stage 6 will pick up. + +Geometry is `shape → TSD.ShapeArchive → TSD.DrawableArchive → geometry`, with +position in field 1 and size in field 2, both `TSP.Point`s of `float` points. +**A size of zero is a box that grows with its text**, not a box of zero height +— the free text box on slide 4 stores `(0, 0)` — so the adapter reports no +measure for it and lets the content decide. + +Slides carry no name in the archive, so they are numbered in presentation +order the way `oldms/presentation` numbers `.ppt` slides. The slide size is the +show's (field 4), 1024×768 points on both fixtures. ## Paragraphs come from the run table @@ -100,6 +144,11 @@ every `\n` in the text. The two agree on both fixtures, but the table is what says so, and `U+2028` is a line break *inside* a paragraph rather than a paragraph boundary. +**The paragraph mark differs by app**: Pages ends a paragraph with `\n` and +Keynote with `\r`. Neither is what says where a paragraph starts — the run +table is — so the mark only decides which trailing byte belongs to the +paragraph it ends. + Run tables count in **UTF-16 code units** while the text is UTF-8; `util::string::utf16_offsets` translates the indices in one pass. An index that lands mid-character is an error, not a rounding. @@ -108,12 +157,16 @@ lands mid-character is an error, not a rounding. anchor is dropped rather than rendered as a glyph — see stage 4. `empty.pages` is the regression that matters at this level: a body storage that -carries no text at all must produce an empty body, not an exception. +carries no text at all must produce an empty body, not an exception. `empty.key` +is its Keynote counterpart — one slide whose placeholders are empty and whose +drawable list is therefore absent, which must come back as one empty slide. ## Not read yet `Index/DocumentStylesheet.iwa` (so `text_root_page_layout` is empty and every -style is the default), drawables and images, `Index/Tables/`, and everything -`PLAN.md` lists as deferred. `password_encrypted()` is not answered either: an -encrypted package is one whose `Index/Document.iwa` does not decompress, which -falls back to reporting the file as a zip. +style is the default), images, `Index/Tables/`, Keynote masters +(`Index/TemplateSlide-*.iwa`, so `slide_master_page` is null) and presenter +notes, drawables anchored in a Pages text flow, and everything `PLAN.md` lists +as deferred. `password_encrypted()` is not answered either: an encrypted +package is one whose `Index/Document.iwa` does not decompress, which falls back +to reporting the file as a zip. diff --git a/src/odr/internal/iwork/PLAN.md b/src/odr/internal/iwork/PLAN.md index a8bbf830..9505fa37 100644 --- a/src/odr/internal/iwork/PLAN.md +++ b/src/odr/internal/iwork/PLAN.md @@ -1,24 +1,28 @@ # iWork plan Where an iwork module goes, and in what order. Written before stage 1; kept -honest as stages land. **Stages 1 and 2 have landed** — see -[`AGENTS.md`](AGENTS.md) for what they decided. Stage 3 is next. +honest as stages land. **Stages 1, 2 and 5 have landed** — see +[`AGENTS.md`](AGENTS.md) for what they decided. Stage 5 was pulled ahead of 3 +and 4 because it needed neither: a slide is a container above the text storage +stage 2 already read. Stage 6 is next. ## Today -A `.pages` opens as a text document and renders its body text. `.numbers` and -`.key` have `FileType` entries and `file_type_table.cpp` rows so a caller can -name them, but no capabilities and no engine behind them: which app wrote a -package is read off its root archive type, and neither has a fixture to pin -that against. - -Two fixtures are committed: -`test/data/input/odr-public/pages/{empty.pages,style-various-1.pages}`, both -written by iWork 13.2 (`Metadata/BuildVersionHistory.plist`). Neither is listed -in `index.csv` — they do not need to be, `TestData` picks up anything the file -type table knows an extension for — and they gained reference output when stage -2 turned `translate_html` on. `style-various-1.pages` carries `Index/Tables/` -and nine files under `Data/`, which is most of the surface below. +A `.pages` opens as a text document and renders its body text; a `.key` opens +as a presentation and renders each slide's text boxes as positioned frames. +`.numbers` has a `FileType` entry and a `file_type_table.cpp` row so a caller +can name it, but no capabilities and no engine behind it. + +Four fixtures are committed: +`test/data/input/odr-public/pages/{empty.pages,style-various-1.pages}`, written +by iWork 13.2, and `test/data/input/odr-public/key/{empty.key, +style-various-1.key}`, written by iWork 14.4 +(`Metadata/BuildVersionHistory.plist`). None is listed in `index.csv` — they do +not need to be, `TestData` picks up anything the file type table knows an +extension for — and each gained reference output when its format turned +`translate_html` on. `style-various-1.pages` carries `Index/Tables/` and nine +files under `Data/`, and `style-various-1.key` a table on its last slide, which +is most of the surface below. ## Spec @@ -240,7 +244,7 @@ against UTF-8 text, which the parser translates in one pass. body flow, so once frames exist it is a different root assembly, not new parsing. -## Stage 5 — Keynote +## Stage 5 — Keynote *(landed)* Cheaper than it looks, and therefore before Numbers: slides are a container above the *same* text storage stage 2 and 3 already read. @@ -251,6 +255,22 @@ above the *same* text storage stage 2 and 3 already read. - `iwork_keynote` gains `.translate_html = true`; needs a `.key` fixture in the public data repo first. +Landed with three deviations: + +- **The root archive does not say which app wrote the package.** The plan + assumed it did, as it does for Pages. `KN.DocumentArchive` and + `TN.DocumentArchive` are both type 1 — ids are namespaced per app — so + Keynote is told from Numbers by the `Slide` components a deck holds. See + `AGENTS.md`. +- **Masters are not read.** `slide_master_page` is null; the + `Index/TemplateSlide-*.iwa` components are there and unopened, which costs + the theme background and nothing a reader misses in text. +- **Frames arrived here rather than in stage 4.** A slide is drawables on a + canvas, so there was no "text and nothing else" shape to land first. The + geometry the drawable already carries is read, which is the piece stage 4 + needs for Pages; what stage 4 still owes is images, shapes and the anchoring + a Pages text flow does. + ## Stage 6 — the tile reader Tables in iWork are stored as **tiles** — row ranges holding packed cell @@ -310,6 +330,10 @@ without a Numbers fixture existing. knows — and reference output was regenerated when stage 2 flipped `translate_html` on. -Stages 5 and 7 each need a fixture that does not exist yet — one `.key` and one -`.numbers` in the public repo. Everything at container level stays inline, per -stage 1. +Stage 7 needs a `.numbers` fixture that does not exist yet. Everything at +container level stays inline, per stage 1. + +The `.key` fixtures were authored on macOS with Keynote 14.4 rather than found: +there is no spec, so a file the app wrote is the only citation available, and +one written to order can carry exactly the shapes a stage needs — a title +slide, a bulleted body, an empty placeholder, a free text box and a table. diff --git a/src/odr/internal/iwork/iwork_archive.cpp b/src/odr/internal/iwork/iwork_archive.cpp index f53a888d..4d817887 100644 --- a/src/odr/internal/iwork/iwork_archive.cpp +++ b/src/odr/internal/iwork/iwork_archive.cpp @@ -73,6 +73,11 @@ Package::Package(const abstract::ReadableFilesystem &filesystem) } } +bool Package::has_component(const std::string &name) const noexcept { + return std::ranges::find(m_component_infos, name, &ComponentInfo::name) != + std::ranges::end(m_component_infos); +} + const Component &Package::component(const std::string &name) { const auto it = std::ranges::find(m_component_infos, name, &ComponentInfo::name); diff --git a/src/odr/internal/iwork/iwork_archive.hpp b/src/odr/internal/iwork/iwork_archive.hpp index a12c4775..ddb17696 100644 --- a/src/odr/internal/iwork/iwork_archive.hpp +++ b/src/odr/internal/iwork/iwork_archive.hpp @@ -50,6 +50,10 @@ class Package final { public: explicit Package(const abstract::ReadableFilesystem &filesystem); + /// Whether the package's component list holds one named @p name. Reads the + /// list only — nothing is decompressed. + [[nodiscard]] bool has_component(const std::string &name) const noexcept; + /// The first component named @p name — a name is not unique. Throws when /// the package holds none. const Component &component(const std::string &name); diff --git a/src/odr/internal/iwork/iwork_document.cpp b/src/odr/internal/iwork/iwork_document.cpp index 5d6848f7..e424d276 100644 --- a/src/odr/internal/iwork/iwork_document.cpp +++ b/src/odr/internal/iwork/iwork_document.cpp @@ -2,25 +2,43 @@ #include #include +#include #include #include #include #include +#include #include namespace odr::internal::iwork { namespace { + std::unique_ptr create_element_adapter(ElementRegistry ®istry); + +ElementIdentifier parse_tree(ElementRegistry ®istry, + const FileType file_type, + const abstract::ReadableFilesystem &files) { + switch (file_type) { + case FileType::iwork_pages: + return parse_pages_tree(registry, files); + case FileType::iwork_keynote: + return parse_keynote_tree(registry, files); + default: + throw UnsupportedFileType(file_type); + } } -Document::Document(std::shared_ptr files) - : internal::Document(FileType::iwork_pages, DocumentType::text, +} // namespace + +Document::Document(const FileType file_type, + std::shared_ptr files) + : internal::Document(file_type, document_type_by_file_type(file_type), std::move(files)) { - m_root_element = parse_pages_tree(m_element_registry, *m_files); + m_root_element = parse_tree(m_element_registry, file_type, *m_files); m_element_adapter = create_element_adapter(m_element_registry); } @@ -51,6 +69,8 @@ namespace { class ElementAdapter final : public abstract::ElementAdapter, public abstract::TextRootAdapter, + public abstract::SlideAdapter, + public abstract::FrameAdapter, public abstract::LineBreakAdapter, public abstract::ParagraphAdapter, public abstract::TextAdapter { @@ -83,19 +103,16 @@ class ElementAdapter final : public abstract::ElementAdapter, return m_registry->element_at(element_id).next_sibling_id; } - [[nodiscard]] bool - element_is_unique(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] bool element_is_unique( + [[maybe_unused]] const ElementIdentifier element_id) const override { return true; } - [[nodiscard]] bool - element_is_self_locatable(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] bool element_is_self_locatable( + [[maybe_unused]] const ElementIdentifier element_id) const override { return true; } - [[nodiscard]] bool - element_is_editable(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] bool element_is_editable( + [[maybe_unused]] const ElementIdentifier element_id) const override { return false; } [[nodiscard]] DocumentPath @@ -112,6 +129,14 @@ class ElementAdapter final : public abstract::ElementAdapter, text_root_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::root ? this : nullptr; } + [[nodiscard]] const SlideAdapter * + slide_adapter(const ElementIdentifier element_id) const override { + return element_type(element_id) == ElementType::slide ? this : nullptr; + } + [[nodiscard]] const FrameAdapter * + frame_adapter(const ElementIdentifier element_id) const override { + return element_type(element_id) == ElementType::frame ? this : nullptr; + } [[nodiscard]] const LineBreakAdapter * line_break_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::line_break ? this : nullptr; @@ -127,31 +152,83 @@ class ElementAdapter final : public abstract::ElementAdapter, // The page geometry sits in the document archive and the styles in // `Index/DocumentStylesheet.iwa`; neither is read yet. - [[nodiscard]] PageLayout - text_root_page_layout(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] PageLayout text_root_page_layout( + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } [[nodiscard]] ElementIdentifier text_root_first_master_page( - const ElementIdentifier element_id) const override { - (void)element_id; + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } - [[nodiscard]] TextStyle - line_break_style(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] std::string + slide_name(const ElementIdentifier element_id) const override { + return m_registry->slide_element_at(element_id).name; + } + [[nodiscard]] PageLayout + slide_page_layout(const ElementIdentifier element_id) const override { + const std::optional &size = + m_registry->slide_element_at(element_id).size; + if (!size.has_value()) { + return {}; + } + return { + .width = points(size->width), + .height = points(size->height), + .print_orientation = {}, + .margin = {}, + }; + } + [[nodiscard]] ElementIdentifier slide_master_page( + [[maybe_unused]] const ElementIdentifier element_id) const override { + // `Index/TemplateSlide-*.iwa` holds the masters; nothing reads them yet. + return null_element_id; + } + + [[nodiscard]] AnchorType frame_anchor_type( + [[maybe_unused]] const ElementIdentifier element_id) const override { + return AnchorType::at_page; + } + [[nodiscard]] std::optional + frame_x(const ElementIdentifier element_id) const override { + return rect_measure(element_id, + [](const ElementRegistry::Rect &r) { return r.x; }); + } + [[nodiscard]] std::optional + frame_y(const ElementIdentifier element_id) const override { + return rect_measure(element_id, + [](const ElementRegistry::Rect &r) { return r.y; }); + } + [[nodiscard]] std::optional + frame_width(const ElementIdentifier element_id) const override { + return rect_measure(element_id, + [](const ElementRegistry::Rect &r) { return r.width; }); + } + [[nodiscard]] std::optional + frame_height(const ElementIdentifier element_id) const override { + return rect_measure( + element_id, [](const ElementRegistry::Rect &r) { return r.height; }); + } + [[nodiscard]] std::optional frame_z_index( + [[maybe_unused]] const ElementIdentifier element_id) const override { + return std::nullopt; + } + [[nodiscard]] GraphicStyle frame_style( + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } - [[nodiscard]] ParagraphStyle - paragraph_style(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] TextStyle line_break_style( + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } - [[nodiscard]] TextStyle - paragraph_text_style(const ElementIdentifier element_id) const override { - (void)element_id; + + [[nodiscard]] ParagraphStyle paragraph_style( + [[maybe_unused]] const ElementIdentifier element_id) const override { + return {}; + } + [[nodiscard]] TextStyle paragraph_text_style( + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } @@ -159,19 +236,41 @@ class ElementAdapter final : public abstract::ElementAdapter, text_content(const ElementIdentifier element_id) const override { return m_registry->text_element_at(element_id).text; } - void text_set_content(const ElementIdentifier element_id, - const std::string &text) const override { - (void)element_id; - (void)text; + void + text_set_content([[maybe_unused]] const ElementIdentifier element_id, + [[maybe_unused]] const std::string &text) const override { throw UnsupportedOperation(); } - [[nodiscard]] TextStyle - text_style(const ElementIdentifier element_id) const override { - (void)element_id; + [[nodiscard]] TextStyle text_style( + [[maybe_unused]] const ElementIdentifier element_id) const override { return {}; } private: + static Measure points(const float value) { + return Measure(value, DynamicUnit("pt")); + } + + /// One side of a frame's rectangle, or nothing where the geometry is missing + /// or zero — a Keynote text box sized to its text stores a zero size, and + /// the renderer does better letting the content decide than with a `0pt` + /// box. + template + [[nodiscard]] std::optional + rect_measure(const ElementIdentifier element_id, + const Selector &select) const { + const std::optional &rect = + m_registry->frame_element_at(element_id).rect; + if (!rect.has_value()) { + return std::nullopt; + } + const float value = select(*rect); + if (value == 0.0F) { + return std::nullopt; + } + return points(value); + } + ElementRegistry *m_registry{nullptr}; }; diff --git a/src/odr/internal/iwork/iwork_document.hpp b/src/odr/internal/iwork/iwork_document.hpp index d73c8b0d..8b734df2 100644 --- a/src/odr/internal/iwork/iwork_document.hpp +++ b/src/odr/internal/iwork/iwork_document.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include @@ -7,10 +9,12 @@ namespace odr::internal::iwork { -/// A `.pages` package, read as a text document. +/// An iWork package, read as the kind of document the app that wrote it +/// makes: a `.pages` as text, a `.key` as a presentation. class Document final : public internal::Document { public: - explicit Document(std::shared_ptr files); + Document(FileType file_type, + std::shared_ptr files); [[nodiscard]] const ElementRegistry &element_registry() const; diff --git a/src/odr/internal/iwork/iwork_element_registry.cpp b/src/odr/internal/iwork/iwork_element_registry.cpp index 2ce6952f..aab2f992 100644 --- a/src/odr/internal/iwork/iwork_element_registry.cpp +++ b/src/odr/internal/iwork/iwork_element_registry.cpp @@ -7,6 +7,8 @@ namespace odr::internal::iwork { void ElementRegistry::clear() noexcept { m_elements.clear(); m_texts.clear(); + m_frames.clear(); + m_slides.clear(); } [[nodiscard]] std::size_t ElementRegistry::size() const noexcept { @@ -29,6 +31,22 @@ ElementRegistry::create_text_element() { return {element_id, element, it->second}; } +std::tuple +ElementRegistry::create_frame_element() { + const auto &[element_id, element] = create_element(ElementType::frame); + auto [it, success] = m_frames.emplace(element_id, Frame{}); + return {element_id, element, it->second}; +} + +std::tuple +ElementRegistry::create_slide_element() { + const auto &[element_id, element] = create_element(ElementType::slide); + auto [it, success] = m_slides.emplace(element_id, Slide{}); + return {element_id, element, it->second}; +} + ElementRegistry::Element & ElementRegistry::element_at(const ElementIdentifier id) { check_element_id(id); @@ -41,6 +59,18 @@ ElementRegistry::text_element_at(const ElementIdentifier id) { return m_texts.at(id); } +ElementRegistry::Frame & +ElementRegistry::frame_element_at(const ElementIdentifier id) { + check_frame_id(id); + return m_frames.at(id); +} + +ElementRegistry::Slide & +ElementRegistry::slide_element_at(const ElementIdentifier id) { + check_slide_id(id); + return m_slides.at(id); +} + const ElementRegistry::Element & ElementRegistry::element_at(const ElementIdentifier id) const { check_element_id(id); @@ -53,6 +83,18 @@ ElementRegistry::text_element_at(const ElementIdentifier id) const { return m_texts.at(id); } +const ElementRegistry::Frame & +ElementRegistry::frame_element_at(const ElementIdentifier id) const { + check_frame_id(id); + return m_frames.at(id); +} + +const ElementRegistry::Slide & +ElementRegistry::slide_element_at(const ElementIdentifier id) const { + check_slide_id(id); + return m_slides.at(id); +} + void ElementRegistry::append_child(const ElementIdentifier parent_id, const ElementIdentifier child_id) { check_element_id(parent_id); @@ -93,4 +135,18 @@ void ElementRegistry::check_text_id(const ElementIdentifier id) const { } } +void ElementRegistry::check_frame_id(const ElementIdentifier id) const { + check_element_id(id); + if (!m_frames.contains(id)) { + throw std::out_of_range("ElementRegistry::check_id: identifier not found"); + } +} + +void ElementRegistry::check_slide_id(const ElementIdentifier id) const { + check_element_id(id); + if (!m_slides.contains(id)) { + throw std::out_of_range("ElementRegistry::check_id: identifier not found"); + } +} + } // namespace odr::internal::iwork diff --git a/src/odr/internal/iwork/iwork_element_registry.hpp b/src/odr/internal/iwork/iwork_element_registry.hpp index a7a7bbaa..c21bd45d 100644 --- a/src/odr/internal/iwork/iwork_element_registry.hpp +++ b/src/odr/internal/iwork/iwork_element_registry.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -13,6 +14,18 @@ namespace odr::internal::iwork { class ElementRegistry final { public: + struct Size final { + float width{}; + float height{}; + }; + + struct Rect final { + float x{}; + float y{}; + float width{}; + float height{}; + }; + struct Element final { ElementIdentifier parent_id{null_element_id}; ElementIdentifier first_child_id{null_element_id}; @@ -26,27 +39,49 @@ class ElementRegistry final { std::string text; }; + /// Where a drawable sits on its page, in points. Absent for one whose + /// geometry we did not find. + struct Frame final { + std::optional rect; + }; + + /// A slide's name and the size of the page it is shown on, in points. + struct Slide final { + std::string name; + std::optional size; + }; + void clear() noexcept; [[nodiscard]] std::size_t size() const noexcept; std::tuple create_element(ElementType type); std::tuple create_text_element(); + std::tuple create_frame_element(); + std::tuple create_slide_element(); [[nodiscard]] Element &element_at(ElementIdentifier id); [[nodiscard]] Text &text_element_at(ElementIdentifier id); + [[nodiscard]] Frame &frame_element_at(ElementIdentifier id); + [[nodiscard]] Slide &slide_element_at(ElementIdentifier id); [[nodiscard]] const Element &element_at(ElementIdentifier id) const; [[nodiscard]] const Text &text_element_at(ElementIdentifier id) const; + [[nodiscard]] const Frame &frame_element_at(ElementIdentifier id) const; + [[nodiscard]] const Slide &slide_element_at(ElementIdentifier id) const; void append_child(ElementIdentifier parent_id, ElementIdentifier child_id); private: std::vector m_elements; std::unordered_map m_texts; + std::unordered_map m_frames; + std::unordered_map m_slides; void check_element_id(ElementIdentifier id) const; void check_text_id(ElementIdentifier id) const; + void check_frame_id(ElementIdentifier id) const; + void check_slide_id(ElementIdentifier id) const; }; } // namespace odr::internal::iwork diff --git a/src/odr/internal/iwork/iwork_file.cpp b/src/odr/internal/iwork/iwork_file.cpp index af1c2c7b..8efbc6fe 100644 --- a/src/odr/internal/iwork/iwork_file.cpp +++ b/src/odr/internal/iwork/iwork_file.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -17,22 +18,27 @@ namespace odr::internal::iwork { namespace { -/// The type of the root archive says which app wrote the package. Only -/// `.pages` is pinned — a `.numbers` or `.key` fixture would be needed to read -/// theirs off, and the extension is not an answer. -FileType file_type_by_archive_type(const std::uint32_t type) { - switch (type) { - case archive_type::pages_document: - return FileType::iwork_pages; - default: - return FileType::unknown; +/// The component Keynote writes one of per slide. A `.numbers` and a `.pages` +/// package hold none — `empty.key` against `empty.numbers` and both `.pages` +/// fixtures (iWork 14.4 / 13.2). +constexpr std::string_view slide_component = "Slide"; + +/// Which app wrote a package whose root archive is @ref +/// archive_type::app_document. Pages has a type id of its own, but Keynote and +/// Numbers both number their root archive 1 — the id space is per app — so the +/// two are told apart by the components the package holds. +FileType app_by_components(const abstract::ReadableFilesystem &filesystem) { + Package package(filesystem); + if (package.has_component(std::string(slide_component))) { + return FileType::iwork_keynote; } + return FileType::unknown; } /// Reads the root archive of the package's `Document` component. The component -/// list in `Index/Metadata.iwa` is not consulted: this runs on every zip a -/// caller opens, and the `Document` component is the one whose file name never -/// carries an identifier suffix. +/// list in `Index/Metadata.iwa` is not consulted for the root archive itself: +/// this runs on every zip a caller opens, and the `Document` component is the +/// one whose file name never carries an identifier suffix. FileType parse_file_type(const abstract::ReadableFilesystem &filesystem) { const std::string data = read_iwa(filesystem, AbsPath("/Index/Document.iwa")); const std::vector objects = read_objects(data); @@ -40,7 +46,18 @@ FileType parse_file_type(const abstract::ReadableFilesystem &filesystem) { throw NoIworkFile(); } - const FileType file_type = file_type_by_archive_type(objects.front().type); + FileType file_type = FileType::unknown; + switch (objects.front().type) { + case archive_type::pages_document: + file_type = FileType::iwork_pages; + break; + case archive_type::app_document: + file_type = app_by_components(filesystem); + break; + default: + break; + } + if (file_type == FileType::unknown) { throw NoIworkFile(); } @@ -79,7 +96,8 @@ bool IworkFile::is_decodable() const noexcept { return true; } std::shared_ptr IworkFile::document() const { switch (file_type()) { case FileType::iwork_pages: - return std::make_shared(m_filesystem); + case FileType::iwork_keynote: + return std::make_shared(file_type(), m_filesystem); default: throw UnsupportedFileType(file_type()); } diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index ea4ce844..09146fb0 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -20,12 +20,44 @@ namespace { /// `U+2028 LINE SEPARATOR` — a line break inside a paragraph. constexpr std::string_view line_separator = "\xe2\x80\xa8"; /// `U+FFFC OBJECT REPLACEMENT CHARACTER` — where a drawable is anchored in the -/// text. Nothing reads drawables yet, so the anchor is dropped rather than -/// rendered as a glyph. +/// text. Nothing reads drawables anchored in a text flow yet, so the anchor is +/// dropped rather than rendered as a glyph. constexpr std::string_view object_replacement = "\xef\xbf\xbc"; +/// The paragraph mark ends the paragraph it belongs to. Pages writes `\n` and +/// Keynote `\r` — the run table is what says where a paragraph starts either +/// way, so this only decides whether the mark is part of the text. +bool is_paragraph_mark(const char c) { return c == '\n' || c == '\r'; } + +/// The object a `TSP.Reference` in field @p number names. +std::optional reference_identifier(const Message &message, + const std::uint32_t number) { + const std::optional bytes = message.bytes_field(number); + if (!bytes.has_value()) { + return {}; + } + return Message(*bytes).number_field(reference::identifier); +} + +/// The objects the repeated `TSP.Reference` field @p number names, in order. +std::vector reference_identifiers(const Message &message, + const std::uint32_t number) { + std::vector result; + for (const Field &field : message.repeated_field(number)) { + if (field.type != WireType::length_delimited) { + throw std::runtime_error("iwork: malformed reference"); + } + if (const std::optional identifier = + Message(field.bytes).number_field(reference::identifier); + identifier.has_value()) { + result.push_back(*identifier); + } + } + return result; +} + /// The character index each paragraph of @p storage starts at. Paragraph -/// boundaries are the run table's rather than every `\n` in the text — the two +/// boundaries are the run table's rather than every mark in the text — the two /// agree today, but the table is what says so. std::vector paragraph_starts(const Message &storage) { std::vector result; @@ -81,6 +113,152 @@ void parse_paragraph(ElementRegistry ®istry, append_text(content); } +/// Appends the paragraphs of a `TSWP.StorageArchive` to @p parent_id. Shared +/// by every place text lives: a Pages body, a Keynote text box, a table cell. +void parse_storage(ElementRegistry ®istry, const ElementIdentifier parent_id, + const Message &storage) { + // the text arrives as a small number of large strings; the run tables index + // it as one + std::string text; + for (const Field &part : storage.repeated_field(text_storage::text)) { + if (part.type != WireType::length_delimited) { + throw std::runtime_error("iwork: malformed text storage"); + } + text += part.bytes; + } + + const std::vector starts = + util::string::utf16_offsets(text, paragraph_starts(storage)); + + const std::string_view body_text(text); + for (std::size_t i = 0; i < starts.size(); ++i) { + const std::size_t begin = starts[i]; + const std::size_t end = i + 1 < starts.size() ? starts[i + 1] : text.size(); + + std::string_view content = body_text.substr(begin, end - begin); + // the paragraph mark belongs to the paragraph it ends, and the last + // paragraph of a body does not carry one + if (!content.empty() && is_paragraph_mark(content.back())) { + content.remove_suffix(1); + } + + auto [paragraph_id, paragraph] = + registry.create_element(ElementType::paragraph); + registry.append_child(parent_id, paragraph_id); + parse_paragraph(registry, paragraph_id, content); + } +} + +/// The root archive of the package's `Document` component, checked against the +/// type the app is expected to write. +const Object &root_archive(Package &package, const std::uint32_t type) { + const std::vector &objects = package.component("Document").objects(); + if (objects.empty() || objects.front().type != type) { + throw std::runtime_error("iwork: no document archive"); + } + return objects.front(); +} + +/// A `TSP.Point`, which iWork writes as two `float`s in points. +std::optional read_size(const Message &message, + const std::uint32_t number) { + const std::optional bytes = message.bytes_field(number); + if (!bytes.has_value()) { + return {}; + } + const Message size(*bytes); + const std::optional width = size.float_field(point::x); + const std::optional height = size.float_field(point::y); + if (!width.has_value() || !height.has_value()) { + return {}; + } + return ElementRegistry::Size{*width, *height}; +} + +/// Where a `TSD.ShapeArchive` sits on its page, in points. +std::optional shape_rect(const Message &shape) { + const std::optional drawable = + shape.bytes_field(shape_archive::drawable); + if (!drawable.has_value()) { + return {}; + } + const Message drawable_message(*drawable); + const std::optional geometry = + drawable_message.bytes_field(drawable_archive::geometry); + if (!geometry.has_value()) { + return {}; + } + const Message geometry_message(*geometry); + + const std::optional position = + read_size(geometry_message, geometry_archive::position); + const std::optional size = + read_size(geometry_message, geometry_archive::size); + if (!position.has_value()) { + return {}; + } + return ElementRegistry::Rect{position->width, position->height, + size.has_value() ? size->width : 0.0F, + size.has_value() ? size->height : 0.0F}; +} + +/// The `TSWP.ShapeArchive` a drawable holds, or nothing when it is a kind we +/// do not read — a table, an image, a group. An unmapped drawable is skipped +/// rather than thrown on: there is no spec, so it is a shape Apple ships and +/// we have not seen. +std::optional text_shape_of(const Object &drawable) { + switch (drawable.type) { + case archive_type::text_shape: + return Message(drawable.payload); + case archive_type::keynote_placeholder: { + const Message placeholder(drawable.payload); + const std::optional shape = + placeholder.bytes_field(placeholder_archive::shape); + if (!shape.has_value()) { + return {}; + } + return Message(*shape); + } + default: + return {}; + } +} + +/// Appends one slide's text boxes to @p slide_id as frames. +void parse_slide(ElementRegistry ®istry, Package &package, + const ElementIdentifier slide_id, const Object &slide) { + const Message slide_message(slide.payload); + + for (const std::uint64_t identifier : + reference_identifiers(slide_message, slide_archive::drawables)) { + const Object &drawable = package.object(identifier); + const std::optional shape = text_shape_of(drawable); + if (!shape.has_value()) { + continue; + } + + const std::optional storage_identifier = + reference_identifier(*shape, text_shape::storage); + if (!storage_identifier.has_value()) { + continue; + } + const Object &storage = package.object(*storage_identifier); + if (storage.type != archive_type::text_storage) { + continue; + } + + auto [frame_id, frame, payload] = registry.create_frame_element(); + if (const std::optional inner = + shape->bytes_field(text_shape::shape); + inner.has_value()) { + payload.rect = shape_rect(Message(*inner)); + } + registry.append_child(slide_id, frame_id); + + parse_storage(registry, frame_id, Message(storage.payload)); + } +} + } // namespace } // namespace odr::internal::iwork @@ -92,60 +270,77 @@ iwork::parse_pages_tree(ElementRegistry ®istry, const abstract::ReadableFilesystem &files) { Package package(files); - const std::vector &objects = package.component("Document").objects(); - if (objects.empty() || objects.front().type != archive_type::pages_document) { - throw std::runtime_error("iwork: no pages document archive"); - } - - const Message document(objects.front().payload); - const std::optional body = - document.bytes_field(document_archive::body_storage); - if (!body.has_value()) { - throw std::runtime_error("iwork: document archive holds no body"); - } + const Message document( + root_archive(package, archive_type::pages_document).payload); const std::optional body_identifier = - Message(*body).number_field(reference::identifier); + reference_identifier(document, document_archive::body_storage); if (!body_identifier.has_value()) { - throw std::runtime_error("iwork: body reference names no object"); + throw std::runtime_error("iwork: document archive holds no body"); } const Object &body_object = package.object(*body_identifier); if (body_object.type != archive_type::text_storage) { throw std::runtime_error("iwork: body is not a text storage"); } - const Message storage(body_object.payload); - // the text arrives as a small number of large strings; the run tables index - // it as one - std::string text; - for (const Field &part : storage.repeated_field(text_storage::text)) { - if (part.type != WireType::length_delimited) { - throw std::runtime_error("iwork: malformed text storage"); - } - text += part.bytes; + auto [root_id, root] = registry.create_element(ElementType::root); + parse_storage(registry, root_id, Message(body_object.payload)); + return root_id; +} + +ElementIdentifier +iwork::parse_keynote_tree(ElementRegistry ®istry, + const abstract::ReadableFilesystem &files) { + Package package(files); + + const Message document( + root_archive(package, archive_type::app_document).payload); + const std::optional show_identifier = + reference_identifier(document, document_archive::show); + if (!show_identifier.has_value()) { + throw std::runtime_error("iwork: document archive holds no show"); } - const std::vector starts = - util::string::utf16_offsets(text, paragraph_starts(storage)); + const Object &show_object = package.object(*show_identifier); + if (show_object.type != archive_type::keynote_show) { + throw std::runtime_error("iwork: show is not a show archive"); + } + const Message show(show_object.payload); + const std::optional slide_size = + read_size(show, show_archive::size); auto [root_id, root] = registry.create_element(ElementType::root); - const std::string_view body_text(text); - for (std::size_t i = 0; i < starts.size(); ++i) { - const std::size_t begin = starts[i]; - const std::size_t end = i + 1 < starts.size() ? starts[i + 1] : text.size(); + const std::optional tree = + show.bytes_field(show_archive::slide_tree); + if (!tree.has_value()) { + return root_id; + } - std::string_view content = body_text.substr(begin, end - begin); - // the paragraph mark belongs to the paragraph it ends, and the last - // paragraph of a body does not carry one - if (content.ends_with('\n')) { - content.remove_suffix(1); + std::size_t number = 0; + for (const std::uint64_t identifier : + reference_identifiers(Message(*tree), slide_tree::nodes)) { + const Object &node = package.object(identifier); + if (node.type != archive_type::keynote_slide_node) { + continue; + } + const std::optional slide_identifier = + reference_identifier(Message(node.payload), slide_node::slide); + if (!slide_identifier.has_value()) { + continue; + } + const Object &slide = package.object(*slide_identifier); + if (slide.type != archive_type::keynote_slide) { + continue; } - auto [paragraph_id, paragraph] = - registry.create_element(ElementType::paragraph); - registry.append_child(root_id, paragraph_id); - parse_paragraph(registry, paragraph_id, content); + auto [slide_id, element, payload] = registry.create_slide_element(); + // slides carry no name in the archive; number them in presentation order + payload.name = "Slide " + std::to_string(++number); + payload.size = slide_size; + registry.append_child(root_id, slide_id); + + parse_slide(registry, package, slide_id, slide); } return root_id; diff --git a/src/odr/internal/iwork/iwork_parser.hpp b/src/odr/internal/iwork/iwork_parser.hpp index 82cbd72c..0ac9cc1e 100644 --- a/src/odr/internal/iwork/iwork_parser.hpp +++ b/src/odr/internal/iwork/iwork_parser.hpp @@ -15,4 +15,10 @@ class ElementRegistry; ElementIdentifier parse_pages_tree(ElementRegistry ®istry, const abstract::ReadableFilesystem &files); +/// Parses the slides of a `.key` package into root → slide → frame → +/// paragraph → text elements. +/// \return the root element id. +ElementIdentifier parse_keynote_tree(ElementRegistry ®istry, + const abstract::ReadableFilesystem &files); + } // namespace odr::internal::iwork diff --git a/src/odr/internal/iwork/iwork_protobuf.cpp b/src/odr/internal/iwork/iwork_protobuf.cpp index bddb1103..c494f1ea 100644 --- a/src/odr/internal/iwork/iwork_protobuf.cpp +++ b/src/odr/internal/iwork/iwork_protobuf.cpp @@ -2,6 +2,7 @@ #include +#include #include namespace odr::internal::iwork { @@ -103,6 +104,14 @@ Message::bytes_field(const std::uint32_t number) const { return field->bytes; } +std::optional Message::float_field(const std::uint32_t number) const { + const std::optional field = this->field(number); + if (!field.has_value() || field->type != WireType::fixed32) { + return {}; + } + return std::bit_cast(static_cast(field->number_value)); +} + } // namespace odr::internal::iwork namespace odr::internal { diff --git a/src/odr/internal/iwork/iwork_protobuf.hpp b/src/odr/internal/iwork/iwork_protobuf.hpp index 85b75a82..32f35057 100644 --- a/src/odr/internal/iwork/iwork_protobuf.hpp +++ b/src/odr/internal/iwork/iwork_protobuf.hpp @@ -49,6 +49,8 @@ class Message final { number_field(std::uint32_t number) const; [[nodiscard]] std::optional bytes_field(std::uint32_t number) const; + /// A `fixed32` field read as the `float` iWork stores geometry in. + [[nodiscard]] std::optional float_field(std::uint32_t number) const; private: std::vector m_fields; diff --git a/src/odr/internal/iwork/iwork_types.hpp b/src/odr/internal/iwork/iwork_types.hpp index 7cf0f962..d8aecea7 100644 --- a/src/odr/internal/iwork/iwork_types.hpp +++ b/src/odr/internal/iwork/iwork_types.hpp @@ -11,20 +11,105 @@ namespace odr::internal::iwork { /// section number, and holds for the iWork version that wrote it — see /// `Metadata/BuildVersionHistory.plist`. A type id that is not here is one we /// have not mapped, which the reader skips rather than throws on. +/// +/// Type ids are namespaced per app: `TP.*` (Pages) starts at 10000 while +/// `KN.*` (Keynote) and `TN.*` (Numbers) both start at 1, so a `.key` and a +/// `.numbers` share every low id — see `which_app` in `iwork_file.cpp`. Only +/// the `TS*` frameworks (text storage, drawables, tables) mean the same thing +/// in all three. namespace archive_type { /// `TP.DocumentArchive`, the root of a `.pages` package. /// `empty.pages Index/Document.iwa` object 1 (iWork 13.2). constexpr std::uint32_t pages_document = 10000; +/// `KN.DocumentArchive` and `TN.DocumentArchive`, the root of a `.key` and of +/// a `.numbers` package alike. +/// `empty.key` / `empty.numbers Index/Document.iwa` object 1 (iWork 14.4). +constexpr std::uint32_t app_document = 1; +/// `KN.ShowArchive`, the deck below a `.key` root. +/// `empty.key Index/Document.iwa` object 32004 (iWork 14.4). +constexpr std::uint32_t keynote_show = 2; +/// `KN.SlideNodeArchive`, one entry of the show's slide tree. +/// `empty.key Index/Document.iwa` object 31723 (iWork 14.4). +constexpr std::uint32_t keynote_slide_node = 4; +/// `KN.SlideArchive`, one slide. +/// `empty.key Index/Slide-31724.iwa` object 31724 (iWork 14.4). +constexpr std::uint32_t keynote_slide = 5; +/// `KN.PlaceholderArchive`, a title or body box a slide inherits from its +/// master. Wraps a @ref text_shape in its first field. +/// `empty.key Index/Slide-31724.iwa` object 31730 (iWork 14.4). +constexpr std::uint32_t keynote_placeholder = 7; /// `TSWP.StorageArchive`, a run of text with its run tables. /// `empty.pages Index/Document.iwa` object 1732514 (iWork 13.2). constexpr std::uint32_t text_storage = 2001; +/// `TSWP.ShapeArchive`, a text box: a drawable plus the storage it holds. +/// `style-various-1.key Index/Slide-32281.iwa` object 32369 (iWork 14.4). +constexpr std::uint32_t text_shape = 2011; } // namespace archive_type namespace document_archive { -/// The body text storage, as a `TSP.Reference`. +/// The body text storage, as a `TSP.Reference`. `TP.DocumentArchive` only. constexpr std::uint32_t body_storage = 4; +/// The `KN.ShowArchive`, as a `TSP.Reference`. `KN.DocumentArchive` only. +constexpr std::uint32_t show = 2; } // namespace document_archive +namespace show_archive { +/// The slide tree, holding the deck's slides in presentation order. +constexpr std::uint32_t slide_tree = 3; +/// The slide size, as a `TSP.Size` in points. +constexpr std::uint32_t size = 4; +} // namespace show_archive + +namespace slide_tree { +/// One `KN.SlideNodeArchive`, as a `TSP.Reference`; repeated. +constexpr std::uint32_t nodes = 2; +} // namespace slide_tree + +namespace slide_node { +/// The `KN.SlideArchive` this node stands for, as a `TSP.Reference`. +constexpr std::uint32_t slide = 2; +} // namespace slide_node + +namespace slide_archive { +/// One drawable on the slide, as a `TSP.Reference`; repeated, in z-order. +/// Placeholders the slide leaves empty are not in this list, which is why it +/// is read rather than the title and body references beside it. +constexpr std::uint32_t drawables = 7; +} // namespace slide_archive + +/// `KN.PlaceholderArchive` — a @ref archive_type::text_shape and the kind of +/// placeholder it stands for. +namespace placeholder_archive { +constexpr std::uint32_t shape = 1; +} // namespace placeholder_archive + +/// `TSWP.ShapeArchive` — the drawable that carries a text storage. +namespace text_shape { +constexpr std::uint32_t shape = 1; +constexpr std::uint32_t storage = 2; +} // namespace text_shape + +/// `TSD.ShapeArchive` — a drawable plus the path it is drawn along. +namespace shape_archive { +constexpr std::uint32_t drawable = 1; +} // namespace shape_archive + +/// `TSD.DrawableArchive` — where a shape sits on its page. +namespace drawable_archive { +constexpr std::uint32_t geometry = 1; +} // namespace drawable_archive + +/// `TSD.GeometryArchive` — position and size in points, both `TSP.Point`. +namespace geometry_archive { +constexpr std::uint32_t position = 1; +constexpr std::uint32_t size = 2; +} // namespace geometry_archive + +namespace point { +constexpr std::uint32_t x = 1; +constexpr std::uint32_t y = 2; +} // namespace point + namespace text_storage { /// The text, in a small number of large strings. constexpr std::uint32_t text = 3; diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 49baef89..196ce4d5 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -94,7 +94,7 @@ open_file_as(const std::shared_ptr &file, const FileType as, throw NoOpenDocumentFile(); } - if (as == FileType::iwork_pages) { + if (as == FileType::iwork_pages || as == FileType::iwork_keynote) { ODR_VERBOSE(logger, "open as iwork"); try { auto zip_file = std::make_unique(file); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f2ea8ae0..dd1cff01 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -64,6 +64,7 @@ add_executable(odr_test "src/internal/iwork/iwork_archive_test.cpp" "src/internal/iwork/iwork_protobuf_test.cpp" "src/internal/iwork/iwork_snappy_test.cpp" + "src/internal/iwork/keynote_test.cpp" "src/internal/iwork/pages_test.cpp" "src/internal/rtf/rtf_document_test.cpp" diff --git a/test/data.cmake b/test/data.cmake index 78fac801..e8f72d71 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -7,7 +7,7 @@ odr_test_data( PATH "input/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.git" - REVISION "5d9a5ca3597b6f6c2c77d8195c3dc42f858c4afc") + REVISION "eba3a23736ee41586bd3a800ff640443163a56ba") odr_test_data( PATH "input/odr-private" @@ -17,7 +17,7 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "99f30175c7228bdba0a872269690cb134240977e") + REVISION "7c77e0ecb4878185854d397328eba9c120372fda") odr_test_data( PATH "reference-output/odr-private" diff --git a/test/src/internal/iwork/iwork_test_util.hpp b/test/src/internal/iwork/iwork_test_util.hpp index c5d80a9d..f695751e 100644 --- a/test/src/internal/iwork/iwork_test_util.hpp +++ b/test/src/internal/iwork/iwork_test_util.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -50,6 +51,28 @@ inline std::string message_field(const std::uint32_t number, return varint((number << 3) | 2) + varint(bytes.size()) + bytes; } +/// A `fixed32` field, which is how iWork writes the `float`s of a geometry. +inline std::string float_field(const std::uint32_t number, const float value) { + const auto bits = std::bit_cast(value); + std::string result = varint((number << 3) | 5); + for (std::uint32_t shift = 0; shift < 32; shift += 8) { + result.push_back(static_cast((bits >> shift) & 0xff)); + } + return result; +} + +/// A `TSP.Reference` to @p identifier, in field @p number. +inline std::string reference_field(const std::uint32_t number, + const std::uint64_t identifier) { + return message_field(number, + number_field(types::reference::identifier, identifier)); +} + +/// A `TSP.Point`, which a size is written as too. +inline std::string point(const float x, const float y) { + return float_field(types::point::x, x) + float_field(types::point::y, y); +} + /// `TSP.ArchiveInfo`: an identifier and one `MessageInfo` per payload message. inline std::string archive_info( const std::uint64_t identifier, @@ -166,6 +189,113 @@ text_storage(const std::string &text, return result; } +/// A `TSWP.ShapeArchive`: a drawable at @p rect holding the text storage +/// @p storage_identifier. +inline std::string text_shape(const std::uint64_t storage_identifier, + const float x, const float y, const float width, + const float height) { + const std::string geometry = + message_field(types::geometry_archive::position, point(x, y)) + + message_field(types::geometry_archive::size, point(width, height)); + const std::string drawable = + message_field(types::drawable_archive::geometry, geometry); + const std::string shape = + message_field(types::shape_archive::drawable, drawable); + return message_field(types::text_shape::shape, shape) + + reference_field(types::text_shape::storage, storage_identifier); +} + +/// One slide of a synthetic deck: the text of each of its boxes, and where the +/// box sits. +struct SlideBox final { + std::string text; + std::optional> paragraphs; + float x{}; + float y{}; + float width{}; + float height{}; + /// Wrapped in a `KN.PlaceholderArchive` rather than standing on its own. + bool placeholder{}; +}; + +/// A `.key` package: a root archive holding a show, whose slide tree names one +/// `KN.SlideNodeArchive` per slide. Carries a `Slide` component, which is what +/// tells a Keynote package from a Numbers one. +inline std::shared_ptr +keynote_package(const std::vector> &slides, + const float slide_width = 1024.0F, + const float slide_height = 768.0F) { + constexpr std::uint64_t show_identifier = 2; + // identifiers are handed out in blocks so a slide's objects never collide + constexpr std::uint64_t slide_block = 100; + + std::string tree; + std::string objects; + std::uint64_t identifier = slide_block; + + for (const std::vector &boxes : slides) { + const std::uint64_t node_identifier = identifier; + const std::uint64_t slide_identifier = identifier + 1; + identifier += slide_block; + + tree += reference_field(types::slide_tree::nodes, node_identifier); + + const std::string node = + reference_field(types::slide_node::slide, slide_identifier); + objects += + object(node_identifier, + {{types::archive_type::keynote_slide_node, node.size()}}, node); + + std::string slide; + std::string drawables; + std::uint64_t box_identifier = slide_identifier + 1; + for (const SlideBox &box : boxes) { + const std::uint64_t drawable_identifier = box_identifier++; + const std::uint64_t storage_identifier = box_identifier++; + + slide += + reference_field(types::slide_archive::drawables, drawable_identifier); + + const std::string shape = + text_shape(storage_identifier, box.x, box.y, box.width, box.height); + const std::string payload = + box.placeholder + ? message_field(types::placeholder_archive::shape, shape) + : shape; + const std::uint32_t type = box.placeholder + ? types::archive_type::keynote_placeholder + : types::archive_type::text_shape; + drawables += + object(drawable_identifier, {{type, payload.size()}}, payload); + + const std::string storage = text_storage(box.text, box.paragraphs); + drawables += object(storage_identifier, + {{types::archive_type::text_storage, storage.size()}}, + storage); + } + + objects += + object(slide_identifier, + {{types::archive_type::keynote_slide, slide.size()}}, slide) + + drawables; + } + + const std::string show = + message_field(types::show_archive::slide_tree, tree) + + message_field(types::show_archive::size, + point(slide_width, slide_height)); + const std::string root = + reference_field(types::document_archive::show, show_identifier); + + const std::string document = + object(1, {{types::archive_type::app_document, root.size()}}, root) + + object(show_identifier, + {{types::archive_type::keynote_show, show.size()}}, show) + + objects; + + return package({{"Document", document}, {"Slide", std::string()}}); +} + /// The identifier a synthetic body storage is filed under. constexpr std::uint64_t body_identifier = 5; diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp new file mode 100644 index 00000000..d701415e --- /dev/null +++ b/test/src/internal/iwork/keynote_test.cpp @@ -0,0 +1,234 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include + +#include +#include + +#include +#include +#include + +using namespace odr; +using odr::test::TestData; +namespace builder = odr::test::iwork; +namespace iwork = odr::internal::iwork; + +namespace { + +/// The text of one frame, a line break reading as a newline and a paragraph +/// boundary as well. +std::string frame_text(const Element frame) { + std::string result; + for (const Element paragraph : frame.children()) { + EXPECT_EQ(paragraph.type(), ElementType::paragraph); + if (!result.empty()) { + result += '\n'; + } + for (const Element child : paragraph.children()) { + if (child.type() == ElementType::line_break) { + result += '\n'; + } else { + result += child.as_text().content(); + } + } + } + return result; +} + +/// The text of every frame of every slide, one vector per slide. +std::vector> slides(const Element root) { + std::vector> result; + + for (const Element slide : root.children()) { + EXPECT_EQ(slide.type(), ElementType::slide); + + std::vector frames; + for (const Element frame : slide.children()) { + EXPECT_EQ(frame.type(), ElementType::frame); + frames.push_back(frame_text(frame)); + } + result.push_back(std::move(frames)); + } + + return result; +} + +Document +keynote_document(const std::vector> &boxes) { + return Document(std::make_shared( + FileType::iwork_keynote, builder::keynote_package(boxes))); +} + +} // namespace + +TEST(IworkKeynote, is_detected_by_content) { + const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); + const std::string path = + TestData::test_file_path("odr-public/key/style-various-1.key"); + + EXPECT_THAT(list_file_types(path, logger), + testing::Contains(FileType::iwork_keynote)); + + const DecodedFile file(path, logger); + EXPECT_EQ(file.file_type(), FileType::iwork_keynote); + EXPECT_EQ(file.file_category(), FileCategory::document); + EXPECT_EQ(file.as_document_file().document_type(), + DocumentType::presentation); +} + +// A deck with one blank slide must come back with one empty slide rather than +// throw: `empty.key`'s title and body placeholders hold no text and are not in +// the slide's drawable list at all. +TEST(IworkKeynote, empty) { + const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); + + const DocumentFile document_file( + TestData::test_file_path("odr-public/key/empty.key"), logger); + EXPECT_EQ(document_file.file_type(), FileType::iwork_keynote); + + const Document document = document_file.document(); + EXPECT_EQ(document.document_type(), DocumentType::presentation); + EXPECT_FALSE(document.is_editable()); + EXPECT_FALSE(document.is_savable(false)); + + EXPECT_EQ(slides(document.root_element()), + (std::vector>{{}})); +} + +TEST(IworkKeynote, slide_text) { + const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); + + const DocumentFile document_file( + TestData::test_file_path("odr-public/key/style-various-1.key"), logger); + + const Document document = document_file.document(); + const std::vector> text = + slides(document.root_element()); + + ASSERT_EQ(text.size(), 4); + EXPECT_EQ(text[0], (std::vector{"Presentation Title", + "A subtitle for the deck"})); + EXPECT_EQ(text[1], + (std::vector{ + "Bullets", "First bullet\nSecond bullet\nThird bullet"})); + // the slide's body placeholder is empty, so it is not on the slide at all + EXPECT_EQ(text[2], (std::vector{"Centred Title"})); + // the table is a drawable this stage does not read; the text box beside it + // is + EXPECT_EQ(text[3], + (std::vector{"A free text box\non a blank slide"})); +} + +TEST(IworkKeynote, slides_are_named_in_presentation_order) { + const DocumentFile document_file( + TestData::test_file_path("odr-public/key/style-various-1.key"), + Logger::null()); + + const Document document = document_file.document(); + + std::vector names; + for (const Element slide : document.root_element().children()) { + names.push_back(slide.as_slide().name()); + } + + EXPECT_EQ(names, (std::vector{"Slide 1", "Slide 2", "Slide 3", + "Slide 4"})); +} + +// The show archive carries the slide size in points; the fixtures are the +// 1024x768 Keynote has defaulted to since the 13 era. +TEST(IworkKeynote, slide_page_layout_comes_from_the_show) { + const DocumentFile document_file( + TestData::test_file_path("odr-public/key/empty.key"), Logger::null()); + + const Document document = document_file.document(); + const Slide slide = (*document.root_element().children().begin()).as_slide(); + const PageLayout layout = slide.page_layout(); + + ASSERT_TRUE(layout.width.has_value()); + ASSERT_TRUE(layout.height.has_value()); + EXPECT_EQ(layout.width->to_string(), "1024pt"); + EXPECT_EQ(layout.height->to_string(), "768pt"); +} + +TEST(IworkKeynote, a_text_box_is_a_frame_where_the_geometry_puts_it) { + const Document document = keynote_document( + {{builder::SlideBox{.text = "boxed\r", + .paragraphs = std::vector{0}, + .x = 100.0F, + .y = 129.0F, + .width = 824.0F, + .height = 260.0F}}}); + + const Element slide = *document.root_element().children().begin(); + const Frame frame = (*slide.children().begin()).as_frame(); + + EXPECT_EQ(frame.anchor_type(), AnchorType::at_page); + ASSERT_TRUE(frame.x().has_value()); + EXPECT_EQ(frame.x()->to_string(), "100pt"); + EXPECT_EQ(frame.y()->to_string(), "129pt"); + EXPECT_EQ(frame.width()->to_string(), "824pt"); + EXPECT_EQ(frame.height()->to_string(), "260pt"); +} + +// Keynote stores a zero size for a box that grows with its text, which is not +// a box of zero height — report no measure and let the content decide. +TEST(IworkKeynote, a_text_box_that_autosizes_reports_no_size) { + const Document document = keynote_document({{builder::SlideBox{ + .text = "grows", .paragraphs = std::nullopt, .x = 478.0F, .y = 384.0F}}}); + + const Element slide = *document.root_element().children().begin(); + const Frame frame = (*slide.children().begin()).as_frame(); + + EXPECT_TRUE(frame.x().has_value()); + EXPECT_FALSE(frame.width().has_value()); + EXPECT_FALSE(frame.height().has_value()); +} + +// A placeholder wraps the same shape a free text box is, one level deeper. +TEST(IworkKeynote, a_placeholder_reads_like_a_text_box) { + const Document document = keynote_document( + {{builder::SlideBox{.text = "titled", .placeholder = true}}}); + + EXPECT_EQ(slides(document.root_element()), + (std::vector>{{"titled"}})); +} + +// Keynote ends a paragraph with `\r` where Pages ends it with `\n`; the run +// table says where the next one starts either way. +TEST(IworkKeynote, a_carriage_return_ends_a_paragraph) { + const Document document = keynote_document({{builder::SlideBox{ + .text = "one\rtwo", .paragraphs = std::vector{0, 4}}}}); + + const Element slide = *document.root_element().children().begin(); + EXPECT_EQ(frame_text(*slide.children().begin()), "one\ntwo"); +} + +TEST(IworkKeynote, a_deck_without_slides_has_an_empty_root) { + const Document document = keynote_document({}); + + EXPECT_EQ(document.root_element().children().begin(), + document.root_element().children().end()); +} + +// A package whose root archive is type 1 is a Keynote one only when it holds +// slide components — Numbers numbers its root archive the same. +TEST(IworkKeynote, a_root_archive_without_slide_components_is_not_keynote) { + const auto files = + builder::pages_package(builder::text_storage("", std::nullopt), + builder::types::archive_type::app_document); + + EXPECT_THROW(iwork::IworkFile{files}, NoIworkFile); +} diff --git a/test/src/internal/iwork/pages_test.cpp b/test/src/internal/iwork/pages_test.cpp index d10c64fe..4d6aab82 100644 --- a/test/src/internal/iwork/pages_test.cpp +++ b/test/src/internal/iwork/pages_test.cpp @@ -58,6 +58,7 @@ Document pages_document( const std::string &text, const std::optional> ¶graph_indices) { return Document(std::make_shared( + FileType::iwork_pages, builder::pages_package(builder::text_storage(text, paragraph_indices)))); } @@ -192,8 +193,8 @@ TEST(Iwork, pages_with_an_empty_paragraph_style_table) { (std::vector{"only"})); } -// Only `.pages` is mapped, so a `.numbers` or `.key` falls back to the zip it -// is rather than being guessed at from its extension. +// A root archive type nothing maps falls back to the zip the package is, +// rather than being guessed at from the extension. TEST(Iwork, unmapped_root_archive_is_not_an_iwork_file) { const auto files = builder::pages_package(builder::text_storage("", std::nullopt), 10001); From 3df72ee60ae4a19ef9b2e8efdae57b31bb2c0644 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 11:35:16 +0200 Subject: [PATCH 02/13] fix(test): assign the placeholder flag rather than designating past it `SlideBox{.text = ..., .placeholder = true}` steps over the geometry members between them, which gcc and clang both reject under `-Werror=missing-field-initializers`; apple clang does not. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CHKtXe9U6EUgikLhNQq7Sm --- test/src/internal/iwork/keynote_test.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index d701415e..15a39c9b 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -199,8 +199,12 @@ TEST(IworkKeynote, a_text_box_that_autosizes_reports_no_size) { // A placeholder wraps the same shape a free text box is, one level deeper. TEST(IworkKeynote, a_placeholder_reads_like_a_text_box) { - const Document document = keynote_document( - {{builder::SlideBox{.text = "titled", .placeholder = true}}}); + // assigned rather than designated: gcc rejects an initializer that steps + // over the geometry to reach the flag behind it + builder::SlideBox box{.text = "titled"}; + box.placeholder = true; + + const Document document = keynote_document({{box}}); EXPECT_EQ(slides(document.root_element()), (std::vector>{{"titled"}})); From c620ec30ae9f2df733521d1bfcc7c027322073c2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 13:56:55 +0200 Subject: [PATCH 03/13] refactor(iwork): look a component up by name as a string_view `has_component` took a `std::string` only to be handed the `string_view` constant its one caller keeps, and `component` repeated the same `ranges::find` line beside it. Both now take a `std::string_view` and share a private `find_`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/iwork_archive.cpp | 17 ++++++++++------- src/odr/internal/iwork/iwork_archive.hpp | 8 ++++++-- src/odr/internal/iwork/iwork_file.cpp | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/odr/internal/iwork/iwork_archive.cpp b/src/odr/internal/iwork/iwork_archive.cpp index 4d817887..7b579265 100644 --- a/src/odr/internal/iwork/iwork_archive.cpp +++ b/src/odr/internal/iwork/iwork_archive.cpp @@ -73,20 +73,23 @@ Package::Package(const abstract::ReadableFilesystem &filesystem) } } -bool Package::has_component(const std::string &name) const noexcept { - return std::ranges::find(m_component_infos, name, &ComponentInfo::name) != - std::ranges::end(m_component_infos); +bool Package::has_component(const std::string_view name) const noexcept { + return find_(name) != std::ranges::end(m_component_infos); } -const Component &Package::component(const std::string &name) { - const auto it = - std::ranges::find(m_component_infos, name, &ComponentInfo::name); +const Component &Package::component(const std::string_view name) { + const auto it = find_(name); if (it == std::ranges::end(m_component_infos)) { - throw std::runtime_error("iwork: no component named " + name); + throw std::runtime_error("iwork: no component named " + std::string(name)); } return load_(*it); } +std::vector::const_iterator +Package::find_(const std::string_view name) const noexcept { + return std::ranges::find(m_component_infos, name, &ComponentInfo::name); +} + const Object &Package::object(const std::uint64_t identifier) { if (const auto it = m_objects.find(identifier); it != m_objects.end()) { return *it->second; diff --git a/src/odr/internal/iwork/iwork_archive.hpp b/src/odr/internal/iwork/iwork_archive.hpp index ddb17696..c1697ffb 100644 --- a/src/odr/internal/iwork/iwork_archive.hpp +++ b/src/odr/internal/iwork/iwork_archive.hpp @@ -52,11 +52,11 @@ class Package final { /// Whether the package's component list holds one named @p name. Reads the /// list only — nothing is decompressed. - [[nodiscard]] bool has_component(const std::string &name) const noexcept; + [[nodiscard]] bool has_component(std::string_view name) const noexcept; /// The first component named @p name — a name is not unique. Throws when /// the package holds none. - const Component &component(const std::string &name); + const Component &component(std::string_view name); /// The object @p identifier names, loading components until it turns up. const Object &object(std::uint64_t identifier); @@ -74,6 +74,10 @@ class Package final { std::deque m_components; std::unordered_map m_objects; + /// The first component list entry named @p name, or `end()`. + [[nodiscard]] std::vector::const_iterator + find_(std::string_view name) const noexcept; + const Component &load_(const ComponentInfo &info); }; diff --git a/src/odr/internal/iwork/iwork_file.cpp b/src/odr/internal/iwork/iwork_file.cpp index 8efbc6fe..8e2348e7 100644 --- a/src/odr/internal/iwork/iwork_file.cpp +++ b/src/odr/internal/iwork/iwork_file.cpp @@ -29,7 +29,7 @@ constexpr std::string_view slide_component = "Slide"; /// two are told apart by the components the package holds. FileType app_by_components(const abstract::ReadableFilesystem &filesystem) { Package package(filesystem); - if (package.has_component(std::string(slide_component))) { + if (package.has_component(slide_component)) { return FileType::iwork_keynote; } return FileType::unknown; From 65f9721994fe722a5c57d9bc26c4b86ea2de5eee Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 13:57:51 +0200 Subject: [PATCH 04/13] refactor(iwork): read a `TSP.Point` as a point The wire type behind a geometry's position is a `TSP.Point`, so reading it into a `Size` made `shape_rect` write `position->width` where it means x. `read_point` returns a `Point`, and `read_size` wraps it for the one field that really is a size. Also points the header's cross-reference at `app_by_components`, which is what the function that tells a `.key` from a `.numbers` is actually called. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/iwork_parser.cpp | 32 ++++++++++++++++++------- src/odr/internal/iwork/iwork_types.hpp | 6 ++--- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 09146fb0..3f17866e 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -159,20 +159,36 @@ const Object &root_archive(Package &package, const std::uint32_t type) { return objects.front(); } -/// A `TSP.Point`, which iWork writes as two `float`s in points. -std::optional read_size(const Message &message, - const std::uint32_t number) { +/// A `TSP.Point`: two `float`s in points, which is how iWork writes a size +/// too. +struct Point final { + float x{}; + float y{}; +}; + +std::optional read_point(const Message &message, + const std::uint32_t number) { const std::optional bytes = message.bytes_field(number); if (!bytes.has_value()) { return {}; } - const Message size(*bytes); - const std::optional width = size.float_field(point::x); - const std::optional height = size.float_field(point::y); - if (!width.has_value() || !height.has_value()) { + const Message point_message(*bytes); + const std::optional x = point_message.float_field(point::x); + const std::optional y = point_message.float_field(point::y); + if (!x.has_value() || !y.has_value()) { + return {}; + } + return Point{*x, *y}; +} + +/// The `TSP.Point` in field @p number, read as the size it stands for. +std::optional read_size(const Message &message, + const std::uint32_t number) { + const std::optional size = read_point(message, number); + if (!size.has_value()) { return {}; } - return ElementRegistry::Size{*width, *height}; + return ElementRegistry::Size{size->x, size->y}; } /// Where a `TSD.ShapeArchive` sits on its page, in points. diff --git a/src/odr/internal/iwork/iwork_types.hpp b/src/odr/internal/iwork/iwork_types.hpp index d8aecea7..ec8c2541 100644 --- a/src/odr/internal/iwork/iwork_types.hpp +++ b/src/odr/internal/iwork/iwork_types.hpp @@ -14,9 +14,9 @@ namespace odr::internal::iwork { /// /// Type ids are namespaced per app: `TP.*` (Pages) starts at 10000 while /// `KN.*` (Keynote) and `TN.*` (Numbers) both start at 1, so a `.key` and a -/// `.numbers` share every low id — see `which_app` in `iwork_file.cpp`. Only -/// the `TS*` frameworks (text storage, drawables, tables) mean the same thing -/// in all three. +/// `.numbers` share every low id — see `app_by_components` in +/// `iwork_file.cpp`. Only the `TS*` frameworks (text storage, drawables, +/// tables) mean the same thing in all three. namespace archive_type { /// `TP.DocumentArchive`, the root of a `.pages` package. /// `empty.pages Index/Document.iwa` object 1 (iWork 13.2). From 97c36237964f5f26b15e798a00e5e19b80951392 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:02 +0200 Subject: [PATCH 05/13] refactor(iwork): let a show with no slide tree fall out of the loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bytes_field` returns an engaged optional for a zero-length field, so the early return only stood for a show archive carrying no field 3 at all — which the builder cannot write and no test reached. Iterating the empty message returns the same empty root with no branch left uncovered. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/iwork_parser.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 3f17866e..65057cc3 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -327,15 +327,13 @@ iwork::parse_keynote_tree(ElementRegistry ®istry, auto [root_id, root] = registry.create_element(ElementType::root); - const std::optional tree = - show.bytes_field(show_archive::slide_tree); - if (!tree.has_value()) { - return root_id; - } + // a show that carries no slide tree is a deck with no slides + const Message tree( + show.bytes_field(show_archive::slide_tree).value_or(std::string_view())); std::size_t number = 0; for (const std::uint64_t identifier : - reference_identifiers(Message(*tree), slide_tree::nodes)) { + reference_identifiers(tree, slide_tree::nodes)) { const Object &node = package.object(identifier); if (node.type != archive_type::keynote_slide_node) { continue; From aff77146f473db584bbb8d4e09380909785d3108 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:09 +0200 Subject: [PATCH 06/13] fix(iwork): report the offset of a frame against the slide edge `rect_measure` read any zero as an absent measure, but only a zero *size* means anything: a text box grown to its text stores one, while a zero x or y is a box flush against the slide's left or top edge. Every other backend reports that as `0pt`, and `frame_style` will silently re-align the frame once styles land. `Rect` now carries the extent as `std::optional`, so the zero-is-autosize reading is made once, in `shape_rect`, rather than encoded as a sentinel there and decoded back in the adapter. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/AGENTS.md | 6 +++-- src/odr/internal/iwork/iwork_document.cpp | 23 ++++++++-------- .../internal/iwork/iwork_element_registry.hpp | 6 +++-- src/odr/internal/iwork/iwork_parser.cpp | 23 +++++++++++----- test/src/internal/iwork/keynote_test.cpp | 27 +++++++++++++++++++ 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/odr/internal/iwork/AGENTS.md b/src/odr/internal/iwork/AGENTS.md index 9e945fd6..8e8d5cc6 100644 --- a/src/odr/internal/iwork/AGENTS.md +++ b/src/odr/internal/iwork/AGENTS.md @@ -128,8 +128,10 @@ Every other drawable kind is skipped: `style-various-1.key` slide 4 carries a Geometry is `shape → TSD.ShapeArchive → TSD.DrawableArchive → geometry`, with position in field 1 and size in field 2, both `TSP.Point`s of `float` points. **A size of zero is a box that grows with its text**, not a box of zero height -— the free text box on slide 4 stores `(0, 0)` — so the adapter reports no -measure for it and lets the content decide. +— the free text box on slide 4 stores `(0, 0)` — so `shape_rect` leaves that +side of the `Rect` unset and the content decides. A *position* of zero is a +box against the slide's edge and is reported as the `0pt` it says: only the +extent is optional. Slides carry no name in the archive, so they are numbered in presentation order the way `oldms/presentation` numbers `.ppt` slides. The slide size is the diff --git a/src/odr/internal/iwork/iwork_document.cpp b/src/odr/internal/iwork/iwork_document.cpp index e424d276..ee321d85 100644 --- a/src/odr/internal/iwork/iwork_document.cpp +++ b/src/odr/internal/iwork/iwork_document.cpp @@ -191,13 +191,15 @@ class ElementAdapter final : public abstract::ElementAdapter, } [[nodiscard]] std::optional frame_x(const ElementIdentifier element_id) const override { - return rect_measure(element_id, - [](const ElementRegistry::Rect &r) { return r.x; }); + return rect_measure(element_id, [](const ElementRegistry::Rect &r) { + return std::optional(r.x); + }); } [[nodiscard]] std::optional frame_y(const ElementIdentifier element_id) const override { - return rect_measure(element_id, - [](const ElementRegistry::Rect &r) { return r.y; }); + return rect_measure(element_id, [](const ElementRegistry::Rect &r) { + return std::optional(r.y); + }); } [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { @@ -251,10 +253,9 @@ class ElementAdapter final : public abstract::ElementAdapter, return Measure(value, DynamicUnit("pt")); } - /// One side of a frame's rectangle, or nothing where the geometry is missing - /// or zero — a Keynote text box sized to its text stores a zero size, and - /// the renderer does better letting the content decide than with a `0pt` - /// box. + /// One side of a frame's rectangle, or nothing where the frame has no + /// geometry or the side itself is absent — which only an extent is, for a + /// box that grows with its text. template [[nodiscard]] std::optional rect_measure(const ElementIdentifier element_id, @@ -264,11 +265,11 @@ class ElementAdapter final : public abstract::ElementAdapter, if (!rect.has_value()) { return std::nullopt; } - const float value = select(*rect); - if (value == 0.0F) { + const std::optional value = select(*rect); + if (!value.has_value()) { return std::nullopt; } - return points(value); + return points(*value); } ElementRegistry *m_registry{nullptr}; diff --git a/src/odr/internal/iwork/iwork_element_registry.hpp b/src/odr/internal/iwork/iwork_element_registry.hpp index c21bd45d..157f0034 100644 --- a/src/odr/internal/iwork/iwork_element_registry.hpp +++ b/src/odr/internal/iwork/iwork_element_registry.hpp @@ -19,11 +19,13 @@ class ElementRegistry final { float height{}; }; + /// A drawable's rectangle, in points. A side of the extent is absent for a + /// box that grows with its text rather than one of zero extent. struct Rect final { float x{}; float y{}; - float width{}; - float height{}; + std::optional width; + std::optional height; }; struct Element final { diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 65057cc3..493f27c6 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -206,16 +206,25 @@ std::optional shape_rect(const Message &shape) { } const Message geometry_message(*geometry); - const std::optional position = - read_size(geometry_message, geometry_archive::position); - const std::optional size = - read_size(geometry_message, geometry_archive::size); + const std::optional position = + read_point(geometry_message, geometry_archive::position); if (!position.has_value()) { return {}; } - return ElementRegistry::Rect{position->width, position->height, - size.has_value() ? size->width : 0.0F, - size.has_value() ? size->height : 0.0F}; + + ElementRegistry::Rect rect{.x = position->x, .y = position->y}; + if (const std::optional size = + read_point(geometry_message, geometry_archive::size); + size.has_value()) { + // a zero side is a box that grows with its text, not a box of zero extent + if (size->x != 0.0F) { + rect.width = size->x; + } + if (size->y != 0.0F) { + rect.height = size->y; + } + } + return rect; } /// The `TSWP.ShapeArchive` a drawable holds, or nothing when it is a kind we diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index 15a39c9b..3d9acfe9 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -16,8 +16,12 @@ #include #include +#include #include +#include +#include #include +#include #include using namespace odr; @@ -177,12 +181,35 @@ TEST(IworkKeynote, a_text_box_is_a_frame_where_the_geometry_puts_it) { EXPECT_EQ(frame.anchor_type(), AnchorType::at_page); ASSERT_TRUE(frame.x().has_value()); + ASSERT_TRUE(frame.y().has_value()); + ASSERT_TRUE(frame.width().has_value()); + ASSERT_TRUE(frame.height().has_value()); EXPECT_EQ(frame.x()->to_string(), "100pt"); EXPECT_EQ(frame.y()->to_string(), "129pt"); EXPECT_EQ(frame.width()->to_string(), "824pt"); EXPECT_EQ(frame.height()->to_string(), "260pt"); } +// A box flush against the left or top edge states an offset of zero, which is +// not the absent measure an unsized box reports. +TEST(IworkKeynote, a_text_box_against_the_slide_edge_is_at_zero) { + const Document document = keynote_document( + {{builder::SlideBox{.text = "flush", + .paragraphs = std::vector{0}, + .x = 0.0F, + .y = 0.0F, + .width = 824.0F, + .height = 260.0F}}}); + + const Element slide = *document.root_element().children().begin(); + const Frame frame = (*slide.children().begin()).as_frame(); + + ASSERT_TRUE(frame.x().has_value()); + ASSERT_TRUE(frame.y().has_value()); + EXPECT_EQ(frame.x()->to_string(), "0pt"); + EXPECT_EQ(frame.y()->to_string(), "0pt"); +} + // Keynote stores a zero size for a box that grows with its text, which is not // a box of zero height — report no measure and let the content decide. TEST(IworkKeynote, a_text_box_that_autosizes_reports_no_size) { From f8142c308e1822df9d6e9906d0d3ebf8e81b245b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:17 +0200 Subject: [PATCH 07/13] fix(iwork): bound what one parse may expand to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An `.iwa` is an object graph walked by identifier, and `Package::object` hands every repeat back from its cache, so a `TSP.Reference` list that names one object a million times costs four bytes a repeat on the wire and a fresh subtree — elements plus a copy of the storage's text — every time it resolves. A few kilobytes of references expand without bound, and because the repeats are siblings rather than ancestors no cycle check sees them. Both tree parsers now spend every element and every byte of text against a `Budget` set far above what an authored document reaches, so a package built to expand throws the `std::runtime_error` callers already handle rather than the allocation the process dies on. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/AGENTS.md | 10 ++++ src/odr/internal/iwork/iwork_parser.cpp | 58 +++++++++++++++++---- test/src/internal/iwork/iwork_test_util.hpp | 10 +++- test/src/internal/iwork/keynote_test.cpp | 36 +++++++++++++ 4 files changed, 103 insertions(+), 11 deletions(-) diff --git a/src/odr/internal/iwork/AGENTS.md b/src/odr/internal/iwork/AGENTS.md index 8e8d5cc6..02a881de 100644 --- a/src/odr/internal/iwork/AGENTS.md +++ b/src/odr/internal/iwork/AGENTS.md @@ -40,6 +40,16 @@ against one before it is known to fit: `snappy_decompress_block` caps its reservation at what the compressed bytes could expand to and checks every tag against what the block has left. +**A reference list is the file's word too.** Because the graph is walked by +identifier and `Package::object` memoises, a `TSP.Reference` list that names +one object a million times costs four bytes a repeat on the wire and a fresh +subtree — elements plus a copy of the storage's text — every time it is +resolved. The repeats are siblings rather than ancestors, so no cycle check +sees them. `parse_pages_tree` and `parse_keynote_tree` therefore spend every +element and every byte of text against a `Budget` set far above what an +authored document reaches, which keeps a package built to expand a thrown +`std::runtime_error` rather than an allocation the process dies on. + ## No new dependencies Two pieces would normally be a conan line each, and both would be wrong. diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 493f27c6..6f9da14b 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -29,6 +30,37 @@ constexpr std::string_view object_replacement = "\xef\xbf\xbc"; /// way, so this only decides whether the mark is part of the text. bool is_paragraph_mark(const char c) { return c == '\n' || c == '\r'; } +/// What one parse may expand to. An `.iwa` is an object graph, so a reference +/// list may name the same object any number of times and `Package::object` +/// hands every repeat back from its cache — a few kilobytes of references +/// would otherwise build elements and copy text without bound. Spending +/// against a budget keeps such a package the thrown error every caller already +/// handles rather than an allocation the process dies on. +class Budget final { +public: + void spend_element() { + if (++m_elements > element_limit) { + throw std::runtime_error("iwork: document holds too many elements"); + } + } + + void spend_text(const std::size_t bytes) { + m_text += bytes; + if (m_text > text_limit) { + throw std::runtime_error("iwork: document holds too much text"); + } + } + +private: + /// Far above what an authored document reaches, and far below what the + /// process cannot hold. + static constexpr std::size_t element_limit = 1'000'000; + static constexpr std::size_t text_limit = std::size_t{64} * 1024 * 1024; + + std::size_t m_elements{}; + std::size_t m_text{}; +}; + /// The object a `TSP.Reference` in field @p number names. std::optional reference_identifier(const Message &message, const std::uint32_t number) { @@ -86,7 +118,7 @@ std::vector paragraph_starts(const Message &storage) { /// Fills @p paragraph_id with the text of one paragraph, breaking it at the /// line separators it holds. -void parse_paragraph(ElementRegistry ®istry, +void parse_paragraph(ElementRegistry ®istry, Budget &budget, const ElementIdentifier paragraph_id, std::string_view content) { const auto append_text = [&](const std::string_view part) { @@ -95,6 +127,8 @@ void parse_paragraph(ElementRegistry ®istry, if (text.empty()) { return; } + budget.spend_element(); + budget.spend_text(text.size()); auto [text_id, element, payload] = registry.create_text_element(); payload.text = std::move(text); registry.append_child(paragraph_id, text_id); @@ -105,6 +139,7 @@ void parse_paragraph(ElementRegistry ®istry, position = content.find(line_separator)) { append_text(content.substr(0, position)); + budget.spend_element(); auto [break_id, element] = registry.create_element(ElementType::line_break); registry.append_child(paragraph_id, break_id); @@ -114,9 +149,9 @@ void parse_paragraph(ElementRegistry ®istry, } /// Appends the paragraphs of a `TSWP.StorageArchive` to @p parent_id. Shared -/// by every place text lives: a Pages body, a Keynote text box, a table cell. -void parse_storage(ElementRegistry ®istry, const ElementIdentifier parent_id, - const Message &storage) { +/// by every place text lives: a Pages body and a Keynote text box. +void parse_storage(ElementRegistry ®istry, Budget &budget, + const ElementIdentifier parent_id, const Message &storage) { // the text arrives as a small number of large strings; the run tables index // it as one std::string text; @@ -142,10 +177,11 @@ void parse_storage(ElementRegistry ®istry, const ElementIdentifier parent_id, content.remove_suffix(1); } + budget.spend_element(); auto [paragraph_id, paragraph] = registry.create_element(ElementType::paragraph); registry.append_child(parent_id, paragraph_id); - parse_paragraph(registry, paragraph_id, content); + parse_paragraph(registry, budget, paragraph_id, content); } } @@ -250,7 +286,7 @@ std::optional text_shape_of(const Object &drawable) { } /// Appends one slide's text boxes to @p slide_id as frames. -void parse_slide(ElementRegistry ®istry, Package &package, +void parse_slide(ElementRegistry ®istry, Budget &budget, Package &package, const ElementIdentifier slide_id, const Object &slide) { const Message slide_message(slide.payload); @@ -272,6 +308,7 @@ void parse_slide(ElementRegistry ®istry, Package &package, continue; } + budget.spend_element(); auto [frame_id, frame, payload] = registry.create_frame_element(); if (const std::optional inner = shape->bytes_field(text_shape::shape); @@ -280,7 +317,7 @@ void parse_slide(ElementRegistry ®istry, Package &package, } registry.append_child(slide_id, frame_id); - parse_storage(registry, frame_id, Message(storage.payload)); + parse_storage(registry, budget, frame_id, Message(storage.payload)); } } @@ -308,8 +345,9 @@ iwork::parse_pages_tree(ElementRegistry ®istry, throw std::runtime_error("iwork: body is not a text storage"); } + Budget budget; auto [root_id, root] = registry.create_element(ElementType::root); - parse_storage(registry, root_id, Message(body_object.payload)); + parse_storage(registry, budget, root_id, Message(body_object.payload)); return root_id; } @@ -334,6 +372,7 @@ iwork::parse_keynote_tree(ElementRegistry ®istry, const std::optional slide_size = read_size(show, show_archive::size); + Budget budget; auto [root_id, root] = registry.create_element(ElementType::root); // a show that carries no slide tree is a deck with no slides @@ -357,13 +396,14 @@ iwork::parse_keynote_tree(ElementRegistry ®istry, continue; } + budget.spend_element(); auto [slide_id, element, payload] = registry.create_slide_element(); // slides carry no name in the archive; number them in presentation order payload.name = "Slide " + std::to_string(++number); payload.size = slide_size; registry.append_child(root_id, slide_id); - parse_slide(registry, package, slide_id, slide); + parse_slide(registry, budget, package, slide_id, slide); } return root_id; diff --git a/test/src/internal/iwork/iwork_test_util.hpp b/test/src/internal/iwork/iwork_test_util.hpp index f695751e..4de4466e 100644 --- a/test/src/internal/iwork/iwork_test_util.hpp +++ b/test/src/internal/iwork/iwork_test_util.hpp @@ -216,6 +216,10 @@ struct SlideBox final { float height{}; /// Wrapped in a `KN.PlaceholderArchive` rather than standing on its own. bool placeholder{}; + /// How many times the slide's drawable list names this box. A deck Keynote + /// wrote names each of its drawables once; a hand-built one may repeat a + /// reference to make the parse expand what a few bytes name. + std::size_t repeats{1}; }; /// A `.key` package: a root archive holding a show, whose slide tree names one @@ -253,8 +257,10 @@ keynote_package(const std::vector> &slides, const std::uint64_t drawable_identifier = box_identifier++; const std::uint64_t storage_identifier = box_identifier++; - slide += - reference_field(types::slide_archive::drawables, drawable_identifier); + for (std::size_t repeat = 0; repeat < box.repeats; ++repeat) { + slide += reference_field(types::slide_archive::drawables, + drawable_identifier); + } const std::string shape = text_shape(storage_identifier, box.x, box.y, box.width, box.height); diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index 3d9acfe9..d9f5df17 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -263,3 +263,39 @@ TEST(IworkKeynote, a_root_archive_without_slide_components_is_not_keynote) { EXPECT_THROW(iwork::IworkFile{files}, NoIworkFile); } + +// A drawable list may name the same object any number of times, and every +// repeat expands into fresh elements holding a fresh copy of its text — a few +// kilobytes of references would otherwise expand without bound. The parse +// throws what every caller already handles instead. +TEST(IworkKeynote, a_repeated_drawable_is_capped_by_the_text_it_copies) { + builder::SlideBox box{.text = std::string(1u << 20, 'a'), + .paragraphs = std::vector{0}}; + box.repeats = 128; + + EXPECT_THAT([&] { keynote_document({{box}}); }, + testing::ThrowsMessage( + testing::HasSubstr("too much text"))); +} + +TEST(IworkKeynote, a_repeated_drawable_is_capped_by_the_elements_it_builds) { + builder::SlideBox box{.text = "short", + .paragraphs = std::vector{0}}; + box.repeats = 600'000; + + EXPECT_THAT([&] { keynote_document({{box}}); }, + testing::ThrowsMessage( + testing::HasSubstr("too many elements"))); +} + +// The cap is far above what a deck reaches, so a list that does name one +// drawable more than once is still read as it is written. +TEST(IworkKeynote, a_drawable_named_more_than_once_is_read_every_time) { + builder::SlideBox box{.text = "again", + .paragraphs = std::vector{0}}; + box.repeats = 3; + + EXPECT_EQ( + slides(keynote_document({{box}}).root_element()), + (std::vector>{{"again", "again", "again"}})); +} From 4eb1a3791315e1d4c0f342cf8bcb4f895637eed5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:17 +0200 Subject: [PATCH 08/13] fix(iwork): decode a package as exactly the type asked for `open_file_as` promises the type its caller names, and every sibling branch checks for it. The iwork branch never had to: the guard admitted `iwork_pages` alone, so the constructed file could only be that. Widening it to `iwork_keynote` made a `.pages` asked for as a deck come back as a text document, with no error, precisely where a caller names the type because detection is ambiguous. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/open_strategy.cpp | 6 +++++- test/src/file_test.cpp | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 196ce4d5..83690234 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -99,7 +99,11 @@ open_file_as(const std::shared_ptr &file, const FileType as, try { auto zip_file = std::make_unique(file); auto filesystem = zip_file->archive()->as_filesystem(); - return std::make_unique(filesystem); + auto iwork_file = std::make_unique(filesystem); + if (iwork_file->file_type() == as) { + return iwork_file; + } + ODR_VERBOSE(logger, "iwork is a different document type"); } catch (...) { ODR_VERBOSE(logger, "failed to open as iwork"); } diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index 90858a60..147e3a21 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -44,6 +44,12 @@ TEST(File, opening_as_the_wrong_document_type_throws) { FileType::office_open_xml_presentation}, {"odr-public/doc/file-sample_100kB.doc", FileType::legacy_word_document, FileType::legacy_excel_worksheets}, + // an iwork package names its own app, so asking for the other one is a + // claim it must refuse rather than answer with what it happens to be + {"odr-public/pages/empty.pages", FileType::iwork_pages, + FileType::iwork_keynote}, + {"odr-public/key/empty.key", FileType::iwork_keynote, + FileType::iwork_pages}, }; for (const auto &[path, is, is_not] : cases) { From e13e52e0b2902cea689f1ccc172631bf03fca0bc Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:24 +0200 Subject: [PATCH 09/13] test(iwork): reach the readers no test reached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `float_field` was the one `Message` accessor with no unit test — its wire-type guard could be deleted and every test still passed — and the empty page layout of a show carrying no size was unreachable, because the builder always wrote one. `keynote_package` now takes an optional size, which is also what its width and height parameters were for; no caller had ever passed them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- .../internal/iwork/iwork_protobuf_test.cpp | 12 ++++++++ test/src/internal/iwork/iwork_test_util.hpp | 28 +++++++++++-------- test/src/internal/iwork/keynote_test.cpp | 20 +++++++++++-- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/test/src/internal/iwork/iwork_protobuf_test.cpp b/test/src/internal/iwork/iwork_protobuf_test.cpp index 6cc44acc..25ac808b 100644 --- a/test/src/internal/iwork/iwork_protobuf_test.cpp +++ b/test/src/internal/iwork/iwork_protobuf_test.cpp @@ -67,6 +67,18 @@ TEST(ProtobufMessage, fixed_fields) { EXPECT_EQ(message.number_field(2), 0x0102030405060708); } +// A geometry's `float`s are `fixed32`; a field of any other wire type is not +// one, which matters where a field number was guessed rather than read. +TEST(ProtobufMessage, float_field) { + const std::string data = key(1, WireType::fixed32) + + std::string{'\x00', '\x00', '\x80', '\xbf'} + + key(2, WireType::varint) + varint(1); + const Message message(data); + EXPECT_EQ(message.float_field(1), -1.0F); + EXPECT_FALSE(message.float_field(2).has_value()); + EXPECT_FALSE(message.float_field(3).has_value()); +} + TEST(ProtobufMessage, bytes_field) { const std::string data = length_delimited(3, "Table of Contents"); const Message message(data); diff --git a/test/src/internal/iwork/iwork_test_util.hpp b/test/src/internal/iwork/iwork_test_util.hpp index 4de4466e..866f7dcf 100644 --- a/test/src/internal/iwork/iwork_test_util.hpp +++ b/test/src/internal/iwork/iwork_test_util.hpp @@ -165,9 +165,8 @@ package(const std::vector> &components) { /// A `TP.DocumentArchive` whose body is the object @p body_identifier. inline std::string document_archive(const std::uint64_t body_identifier) { - return message_field( - types::document_archive::body_storage, - number_field(types::reference::identifier, body_identifier)); + return reference_field(types::document_archive::body_storage, + body_identifier); } /// A `TSWP.StorageArchive`: @p text and a paragraph style run table over the @@ -222,13 +221,19 @@ struct SlideBox final { std::size_t repeats{1}; }; +/// The size of a synthetic deck's slides, in points. +struct SlideSize final { + float width{1024.0F}; + float height{768.0F}; +}; + /// A `.key` package: a root archive holding a show, whose slide tree names one -/// `KN.SlideNodeArchive` per slide. Carries a `Slide` component, which is what -/// tells a Keynote package from a Numbers one. +/// `KN.SlideNodeArchive` per slide. `std::nullopt` writes a show with no size +/// at all, which is not one of zero size. Carries a `Slide` component, which +/// is what tells a Keynote package from a Numbers one. inline std::shared_ptr keynote_package(const std::vector> &slides, - const float slide_width = 1024.0F, - const float slide_height = 768.0F) { + const std::optional &slide_size = SlideSize{}) { constexpr std::uint64_t show_identifier = 2; // identifiers are handed out in blocks so a slide's objects never collide constexpr std::uint64_t slide_block = 100; @@ -286,10 +291,11 @@ keynote_package(const std::vector> &slides, drawables; } - const std::string show = - message_field(types::show_archive::slide_tree, tree) + - message_field(types::show_archive::size, - point(slide_width, slide_height)); + std::string show = message_field(types::show_archive::slide_tree, tree); + if (slide_size.has_value()) { + show += message_field(types::show_archive::size, + point(slide_size->width, slide_size->height)); + } const std::string root = reference_field(types::document_archive::show, show_identifier); diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index d9f5df17..08c94dad 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -69,10 +69,11 @@ std::vector> slides(const Element root) { return result; } -Document -keynote_document(const std::vector> &boxes) { +Document keynote_document( + const std::vector> &boxes, + const std::optional &size = builder::SlideSize{}) { return Document(std::make_shared( - FileType::iwork_keynote, builder::keynote_package(boxes))); + FileType::iwork_keynote, builder::keynote_package(boxes, size))); } } // namespace @@ -224,6 +225,19 @@ TEST(IworkKeynote, a_text_box_that_autosizes_reports_no_size) { EXPECT_FALSE(frame.height().has_value()); } +// The size is the show's, and a show that carries none leaves the layout to +// the renderer rather than laying a slide out at zero. +TEST(IworkKeynote, a_show_without_a_size_has_no_page_layout) { + const Document document = + keynote_document({{builder::SlideBox{.text = "sizeless"}}}, std::nullopt); + + const Slide slide = (*document.root_element().children().begin()).as_slide(); + const PageLayout layout = slide.page_layout(); + + EXPECT_FALSE(layout.width.has_value()); + EXPECT_FALSE(layout.height.has_value()); +} + // A placeholder wraps the same shape a free text box is, one level deeper. TEST(IworkKeynote, a_placeholder_reads_like_a_text_box) { // assigned rather than designated: gcc rejects an initializer that steps From a37e567c164eecdcffba57e2fc160775f8197854 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 14:02:24 +0200 Subject: [PATCH 10/13] test(iwork): pin keynote detection against the numbers fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `KN.DocumentArchive` and `TN.DocumentArchive` are both type 1, so the component list is what tells a `.key` from a `.numbers` — the one rule the detection design rests on, and it was checked by hand. The only committed negative was a synthetic one-component package, which says nothing about how Numbers names its components. The real files were cited by name in `iwork_types.hpp` and `AGENTS.md` but sat one commit past the `input/odr-public` pin, so a fresh checkout had neither the fixture nor a test opening it. Advances the pin and adds the negative over both. They render nothing — `iwork_numbers` declares no `open` capability, so the html sweep skips them — and the reference output the branch already renders is unchanged, so that pin stays put. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UJCLtTYj1yxBmgE2bW5euM --- src/odr/internal/iwork/AGENTS.md | 7 +++--- src/odr/internal/iwork/PLAN.md | 29 ++++++++++++++---------- test/data.cmake | 2 +- test/src/internal/iwork/keynote_test.cpp | 15 ++++++++++++ 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/odr/internal/iwork/AGENTS.md b/src/odr/internal/iwork/AGENTS.md index 02a881de..819912ac 100644 --- a/src/odr/internal/iwork/AGENTS.md +++ b/src/odr/internal/iwork/AGENTS.md @@ -108,9 +108,10 @@ once you already know the app. Only the `TS*` frameworks — `TSWP` text storage What does tell them apart is the **component list**: Keynote writes one `Slide` component per slide and neither Numbers nor Pages writes any. Checked against -all six fixtures. That costs `Index/Metadata.iwa`, which detection otherwise -avoids reading, so it is only reached once the root archive has already come -back as type 1. +all six fixtures, and pinned by the negative the rule rests on — +`IworkKeynote.a_numbers_package_is_not_keynote` opens both `.numbers` files. +That costs `Index/Metadata.iwa`, which detection otherwise avoids reading, so +it is only reached once the root archive has already come back as type 1. `iwork_numbers` still has a `file_type_table.cpp` row with no capabilities so a caller can name it and hand a file picker its MIME types; nothing decodes one diff --git a/src/odr/internal/iwork/PLAN.md b/src/odr/internal/iwork/PLAN.md index 9505fa37..be5b014d 100644 --- a/src/odr/internal/iwork/PLAN.md +++ b/src/odr/internal/iwork/PLAN.md @@ -13,16 +13,18 @@ as a presentation and renders each slide's text boxes as positioned frames. `.numbers` has a `FileType` entry and a `file_type_table.cpp` row so a caller can name it, but no capabilities and no engine behind it. -Four fixtures are committed: +Six fixtures are committed: `test/data/input/odr-public/pages/{empty.pages,style-various-1.pages}`, written -by iWork 13.2, and `test/data/input/odr-public/key/{empty.key, -style-various-1.key}`, written by iWork 14.4 -(`Metadata/BuildVersionHistory.plist`). None is listed in `index.csv` — they do -not need to be, `TestData` picks up anything the file type table knows an -extension for — and each gained reference output when its format turned -`translate_html` on. `style-various-1.pages` carries `Index/Tables/` and nine -files under `Data/`, and `style-various-1.key` a table on its last slide, which -is most of the surface below. +by iWork 13.2, and `test/data/input/odr-public/{key/{empty.key, +style-various-1.key},numbers/{empty.numbers,style-various-1.numbers}}`, written +by iWork 14.4 (`Metadata/BuildVersionHistory.plist`). None is listed in +`index.csv` — they do not need to be, `TestData` picks up anything the file +type table knows an extension for — and each gained reference output when its +format turned `translate_html` on. Nothing decodes the `.numbers` pair yet; +they are what pins the Keynote-versus-Numbers detection rule. +`style-various-1.pages` carries `Index/Tables/` and nine files under `Data/`, +and `style-various-1.key` a table on its last slide, which is most of the +surface below. ## Spec @@ -281,7 +283,7 @@ have not mapped must degrade to an empty cell rather than a wrong one. Do this for **Pages tables first** (`Table`, `TableRow`, `TableCell`), because `style-various-1.pages` already carries `Index/Tables/` and exercises the reader -without a Numbers fixture existing. +without any Numbers archive being mapped. ## Stage 7 — Numbers @@ -330,8 +332,11 @@ without a Numbers fixture existing. knows — and reference output was regenerated when stage 2 flipped `translate_html` on. -Stage 7 needs a `.numbers` fixture that does not exist yet. Everything at -container level stays inline, per stage 1. +`empty.numbers` and `style-various-1.numbers` are in +`test/data/input/odr-public/numbers/`. Stage 7 is what will decode them; today +they are the negative that pins detection +(`IworkKeynote.a_numbers_package_is_not_keynote`). Everything at container +level stays inline, per stage 1. The `.key` fixtures were authored on macOS with Keynote 14.4 rather than found: there is no spec, so a file the app wrote is the only citation available, and diff --git a/test/data.cmake b/test/data.cmake index e8f72d71..b4b9df26 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -7,7 +7,7 @@ odr_test_data( PATH "input/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.git" - REVISION "eba3a23736ee41586bd3a800ff640443163a56ba") + REVISION "c70c296ebe08ba4e109312dcbf55227206b44eaa") odr_test_data( PATH "input/odr-private" diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index 08c94dad..4de43695 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -313,3 +313,18 @@ TEST(IworkKeynote, a_drawable_named_more_than_once_is_read_every_time) { slides(keynote_document({{box}}).root_element()), (std::vector>{{"again", "again", "again"}})); } + +// `KN.DocumentArchive` and `TN.DocumentArchive` are both type 1, so the real +// `.numbers` fixtures are what pins the component list telling the two apart. +TEST(IworkKeynote, a_numbers_package_is_not_keynote) { + for (const char *name : {"empty.numbers", "style-various-1.numbers"}) { + const std::string path = + TestData::test_file_path(std::string("odr-public/numbers/") + name); + + EXPECT_THAT(list_file_types(path, Logger::null()), + testing::Not(testing::Contains(FileType::iwork_keynote))); + EXPECT_THROW(std::ignore = + open(path, FileType::iwork_keynote, Logger::null()), + UnknownFileType); + } +} From dbaabdf768885641e5e93f2629819f1fd975f77e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 19:34:56 +0200 Subject: [PATCH 11/13] fix(iwork): default the members a designated initialiser skips gcc's -Wmissing-field-initializers fires on a designated initialiser that omits a member with no default member initialiser, and the build treats it as an error: `Rect{.x = ..., .y = ...}` left `width` and `height` unnamed. Clang and MSVC say nothing, so only the gcc-14 job caught it. `SlideBox::paragraphs` is the same shape, initialised the same partial way; the with-tests jobs are clang and MSVC only, so it is latent rather than failing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HZ7jMy9qsh5CofYxiAncUG --- src/odr/internal/iwork/iwork_element_registry.hpp | 4 ++-- test/src/internal/iwork/iwork_test_util.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/odr/internal/iwork/iwork_element_registry.hpp b/src/odr/internal/iwork/iwork_element_registry.hpp index 157f0034..6f7d1c75 100644 --- a/src/odr/internal/iwork/iwork_element_registry.hpp +++ b/src/odr/internal/iwork/iwork_element_registry.hpp @@ -24,8 +24,8 @@ class ElementRegistry final { struct Rect final { float x{}; float y{}; - std::optional width; - std::optional height; + std::optional width{}; + std::optional height{}; }; struct Element final { diff --git a/test/src/internal/iwork/iwork_test_util.hpp b/test/src/internal/iwork/iwork_test_util.hpp index 866f7dcf..ddbe83bf 100644 --- a/test/src/internal/iwork/iwork_test_util.hpp +++ b/test/src/internal/iwork/iwork_test_util.hpp @@ -208,7 +208,7 @@ inline std::string text_shape(const std::uint64_t storage_identifier, /// box sits. struct SlideBox final { std::string text; - std::optional> paragraphs; + std::optional> paragraphs{}; float x{}; float y{}; float width{}; From d42675865ff91c5e5dc3975e4b750bc17e7021f7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 19:35:04 +0200 Subject: [PATCH 12/13] refactor(iwork): write a whole float literal as the integer it is `0.0F` and `1024.0F` say nothing `0` and `1024` do not: the conversion is exact for every value here, so the suffix is noise. No other float literal in `src/odr` carries one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HZ7jMy9qsh5CofYxiAncUG --- src/odr/internal/iwork/iwork_parser.cpp | 4 ++-- .../src/internal/iwork/iwork_protobuf_test.cpp | 2 +- test/src/internal/iwork/iwork_test_util.hpp | 4 ++-- test/src/internal/iwork/keynote_test.cpp | 18 +++++++++--------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 6f9da14b..54ed34c8 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -253,10 +253,10 @@ std::optional shape_rect(const Message &shape) { read_point(geometry_message, geometry_archive::size); size.has_value()) { // a zero side is a box that grows with its text, not a box of zero extent - if (size->x != 0.0F) { + if (size->x != 0) { rect.width = size->x; } - if (size->y != 0.0F) { + if (size->y != 0) { rect.height = size->y; } } diff --git a/test/src/internal/iwork/iwork_protobuf_test.cpp b/test/src/internal/iwork/iwork_protobuf_test.cpp index 25ac808b..28a56d54 100644 --- a/test/src/internal/iwork/iwork_protobuf_test.cpp +++ b/test/src/internal/iwork/iwork_protobuf_test.cpp @@ -74,7 +74,7 @@ TEST(ProtobufMessage, float_field) { std::string{'\x00', '\x00', '\x80', '\xbf'} + key(2, WireType::varint) + varint(1); const Message message(data); - EXPECT_EQ(message.float_field(1), -1.0F); + EXPECT_EQ(message.float_field(1), -1); EXPECT_FALSE(message.float_field(2).has_value()); EXPECT_FALSE(message.float_field(3).has_value()); } diff --git a/test/src/internal/iwork/iwork_test_util.hpp b/test/src/internal/iwork/iwork_test_util.hpp index ddbe83bf..a9c5e810 100644 --- a/test/src/internal/iwork/iwork_test_util.hpp +++ b/test/src/internal/iwork/iwork_test_util.hpp @@ -223,8 +223,8 @@ struct SlideBox final { /// The size of a synthetic deck's slides, in points. struct SlideSize final { - float width{1024.0F}; - float height{768.0F}; + float width{1024}; + float height{768}; }; /// A `.key` package: a root archive holding a show, whose slide tree names one diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index 4de43695..7c2a2668 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -172,10 +172,10 @@ TEST(IworkKeynote, a_text_box_is_a_frame_where_the_geometry_puts_it) { const Document document = keynote_document( {{builder::SlideBox{.text = "boxed\r", .paragraphs = std::vector{0}, - .x = 100.0F, - .y = 129.0F, - .width = 824.0F, - .height = 260.0F}}}); + .x = 100, + .y = 129, + .width = 824, + .height = 260}}}); const Element slide = *document.root_element().children().begin(); const Frame frame = (*slide.children().begin()).as_frame(); @@ -197,10 +197,10 @@ TEST(IworkKeynote, a_text_box_against_the_slide_edge_is_at_zero) { const Document document = keynote_document( {{builder::SlideBox{.text = "flush", .paragraphs = std::vector{0}, - .x = 0.0F, - .y = 0.0F, - .width = 824.0F, - .height = 260.0F}}}); + .x = 0, + .y = 0, + .width = 824, + .height = 260}}}); const Element slide = *document.root_element().children().begin(); const Frame frame = (*slide.children().begin()).as_frame(); @@ -215,7 +215,7 @@ TEST(IworkKeynote, a_text_box_against_the_slide_edge_is_at_zero) { // a box of zero height — report no measure and let the content decide. TEST(IworkKeynote, a_text_box_that_autosizes_reports_no_size) { const Document document = keynote_document({{builder::SlideBox{ - .text = "grows", .paragraphs = std::nullopt, .x = 478.0F, .y = 384.0F}}}); + .text = "grows", .paragraphs = std::nullopt, .x = 478, .y = 384}}}); const Element slide = *document.root_element().children().begin(); const Frame frame = (*slide.children().begin()).as_frame(); From fbffebb7da918f493264b393a2bf03d4be8b6b5e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 25 Aug 2026 19:35:32 +0200 Subject: [PATCH 13/13] refactor(iwork): split the `TSWP` text layer into its own translation unit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `iwork_parser.cpp` held the container spines and the text reader they share. Splitting them along Apple's own seam — `TSWP` storage, paragraph run tables and the line separators inside a paragraph move to `iwork_text.cpp` — leaves the parser the three per-app spines and the drawable geometry above them. `Budget` moves with neither: the spines spend for slides and frames as the text layer spends for paragraphs and runs, so it gets its own header. This is the seam the module keeps splitting on as stages land — `TSD` drawables, `TST` tiles, `TSS` styles each earn a file, and no split follows app lines, because `iwork/` stays flat with one registry and one document for all three apps. `PLAN.md` and `AGENTS.md` carry the argument: Apple factored the format by framework, so a `.pages`, a `.key` and a `.numbers` share everything below their spine — including the tile reader stage 6 writes for Pages and stage 7 hands to Numbers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HZ7jMy9qsh5CofYxiAncUG --- CMakeLists.txt | 2 + src/odr/internal/iwork/AGENTS.md | 15 +++ src/odr/internal/iwork/PLAN.md | 29 +++++ src/odr/internal/iwork/iwork_budget.cpp | 20 ++++ src/odr/internal/iwork/iwork_budget.hpp | 28 +++++ src/odr/internal/iwork/iwork_parser.cpp | 143 +----------------------- src/odr/internal/iwork/iwork_text.cpp | 136 ++++++++++++++++++++++ src/odr/internal/iwork/iwork_text.hpp | 15 +++ 8 files changed, 247 insertions(+), 141 deletions(-) create mode 100644 src/odr/internal/iwork/iwork_budget.cpp create mode 100644 src/odr/internal/iwork/iwork_budget.hpp create mode 100644 src/odr/internal/iwork/iwork_text.cpp create mode 100644 src/odr/internal/iwork/iwork_text.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 32e0a8bb..f83ef7b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -153,12 +153,14 @@ set(ODR_SOURCE_FILES "src/odr/internal/html/xml_file.cpp" "src/odr/internal/iwork/iwork_archive.cpp" + "src/odr/internal/iwork/iwork_budget.cpp" "src/odr/internal/iwork/iwork_document.cpp" "src/odr/internal/iwork/iwork_element_registry.cpp" "src/odr/internal/iwork/iwork_file.cpp" "src/odr/internal/iwork/iwork_parser.cpp" "src/odr/internal/iwork/iwork_protobuf.cpp" "src/odr/internal/iwork/iwork_snappy.cpp" + "src/odr/internal/iwork/iwork_text.cpp" "src/odr/internal/json/json_file.cpp" "src/odr/internal/json/json_util.cpp" diff --git a/src/odr/internal/iwork/AGENTS.md b/src/odr/internal/iwork/AGENTS.md index 819912ac..fecb7b3b 100644 --- a/src/odr/internal/iwork/AGENTS.md +++ b/src/odr/internal/iwork/AGENTS.md @@ -10,6 +10,21 @@ renders its paragraphs; a `.key` opens as a presentation and renders each slide's text boxes as frames where the geometry puts them. Everything else in `PLAN.md` is still ahead. +## The files split by framework, not by app + +`iwork/` is flat, and one `ElementRegistry`, one `Document` and one `IworkFile` +serve all three apps. That is `odf/`'s shape rather than `ooxml/`'s, for a +reason the two analogies do not quite carry: Apple factored the format by +**framework**, so `TSWP` text, `TSD` drawables, `TST` tables and `TSS` styles +mean the same thing in a `.pages`, a `.key` and a `.numbers`, and only the +spine above them is per-app. Parsing splits along those seams — `iwork_text.cpp` +is the `TSWP` layer, `iwork_parser.cpp` the three spines that call it — never +along app lines. `PLAN.md` carries the argument and the files each stage adds. + +`Budget` has its own header because it is a parse-wide meter, not a text one: +the spines spend for slides and frames as the text layer spends for paragraphs +and runs. + ## There is no spec, so a fixture is the citation Apple has never published the `.proto` schemas and nothing is vendored under diff --git a/src/odr/internal/iwork/PLAN.md b/src/odr/internal/iwork/PLAN.md index be5b014d..ff354dc5 100644 --- a/src/odr/internal/iwork/PLAN.md +++ b/src/odr/internal/iwork/PLAN.md @@ -68,6 +68,35 @@ type, exactly as `odf::OpenDocumentFile` (`odf_file.hpp:21`) does for the four opendocument types — same constructor over an `abstract::ReadableFilesystem`, same `document()` dispatch. +**And the files split by framework, not by app.** `ooxml/` gives each format a +directory because Microsoft factored by app: WordprocessingML and SpreadsheetML +share nothing below the package, so `text/`, `presentation/` and `spreadsheet/` +each carry their own parser, registry and document. Apple factored by +*framework* instead — `TSWP` text, `TSD` drawables, `TST` tables, `TSS` styles +mean the same thing in all three apps, and only the spine above them +(`TP.DocumentArchive` → body, `KN.ShowArchive` → slides, `TN.DocumentArchive` → +sheets) is per-app. So `iwork/` stays flat, with **one** `ElementRegistry`, one +`Document` and one `IworkFile`, and splits its parsing along Apple's seams: + +| File | Framework | +|---|---| +| `iwork_text.cpp` | `TSWP` storage → paragraphs *(landed)* | +| `iwork_drawable.cpp` | `TSD` geometry, shapes, images *(stage 4)* | +| `iwork_table.cpp` | `TST` tiles → table, row, cell *(stage 6)* | +| `iwork_style.cpp` | `TSS` property-set inheritance *(stage 3)* | +| `iwork_parser.cpp` | the three spines, calling the above | + +The evidence is stage 6: the tile reader is written for **Pages** tables, a +`.key` slide carries a `TST.TableInfoArchive` too, and stage 7 puts Numbers on +top of the same reader. The most expensive component still ahead is shared by +all three apps, which an app-shaped split would either duplicate or push into a +`common/` holding most of the module. + +Only `iwork_text.cpp` and the `Budget` it spends against are split out today — +the rest follows the stage that writes it. Merging three registries later is +not mechanical; splitting one is, so the unified side is the cheap side to be +wrong on. + **No new dependencies.** Two pieces would normally be a conan line each, and both are wrong here: diff --git a/src/odr/internal/iwork/iwork_budget.cpp b/src/odr/internal/iwork/iwork_budget.cpp new file mode 100644 index 00000000..5c14aa5a --- /dev/null +++ b/src/odr/internal/iwork/iwork_budget.cpp @@ -0,0 +1,20 @@ +#include + +#include + +namespace odr::internal::iwork { + +void Budget::spend_element() { + if (++m_elements > element_limit) { + throw std::runtime_error("iwork: document holds too many elements"); + } +} + +void Budget::spend_text(const std::size_t bytes) { + m_text += bytes; + if (m_text > text_limit) { + throw std::runtime_error("iwork: document holds too much text"); + } +} + +} // namespace odr::internal::iwork diff --git a/src/odr/internal/iwork/iwork_budget.hpp b/src/odr/internal/iwork/iwork_budget.hpp new file mode 100644 index 00000000..f0b28b95 --- /dev/null +++ b/src/odr/internal/iwork/iwork_budget.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include + +namespace odr::internal::iwork { + +/// What one parse may expand to. An `.iwa` is an object graph, so a reference +/// list may name the same object any number of times and `Package::object` +/// hands every repeat back from its cache — a few kilobytes of references +/// would otherwise build elements and copy text without bound. Spending +/// against a budget keeps such a package the thrown error every caller already +/// handles rather than an allocation the process dies on. +class Budget final { +public: + void spend_element(); + void spend_text(std::size_t bytes); + +private: + /// Far above what an authored document reaches, and far below what the + /// process cannot hold. + static constexpr std::size_t element_limit = 1'000'000; + static constexpr std::size_t text_limit = std::size_t{64} * 1024 * 1024; + + std::size_t m_elements{}; + std::size_t m_text{}; +}; + +} // namespace odr::internal::iwork diff --git a/src/odr/internal/iwork/iwork_parser.cpp b/src/odr/internal/iwork/iwork_parser.cpp index 54ed34c8..e332e96d 100644 --- a/src/odr/internal/iwork/iwork_parser.cpp +++ b/src/odr/internal/iwork/iwork_parser.cpp @@ -1,10 +1,11 @@ #include #include +#include #include #include +#include #include -#include #include #include @@ -18,49 +19,6 @@ namespace odr::internal::iwork { namespace { -/// `U+2028 LINE SEPARATOR` — a line break inside a paragraph. -constexpr std::string_view line_separator = "\xe2\x80\xa8"; -/// `U+FFFC OBJECT REPLACEMENT CHARACTER` — where a drawable is anchored in the -/// text. Nothing reads drawables anchored in a text flow yet, so the anchor is -/// dropped rather than rendered as a glyph. -constexpr std::string_view object_replacement = "\xef\xbf\xbc"; - -/// The paragraph mark ends the paragraph it belongs to. Pages writes `\n` and -/// Keynote `\r` — the run table is what says where a paragraph starts either -/// way, so this only decides whether the mark is part of the text. -bool is_paragraph_mark(const char c) { return c == '\n' || c == '\r'; } - -/// What one parse may expand to. An `.iwa` is an object graph, so a reference -/// list may name the same object any number of times and `Package::object` -/// hands every repeat back from its cache — a few kilobytes of references -/// would otherwise build elements and copy text without bound. Spending -/// against a budget keeps such a package the thrown error every caller already -/// handles rather than an allocation the process dies on. -class Budget final { -public: - void spend_element() { - if (++m_elements > element_limit) { - throw std::runtime_error("iwork: document holds too many elements"); - } - } - - void spend_text(const std::size_t bytes) { - m_text += bytes; - if (m_text > text_limit) { - throw std::runtime_error("iwork: document holds too much text"); - } - } - -private: - /// Far above what an authored document reaches, and far below what the - /// process cannot hold. - static constexpr std::size_t element_limit = 1'000'000; - static constexpr std::size_t text_limit = std::size_t{64} * 1024 * 1024; - - std::size_t m_elements{}; - std::size_t m_text{}; -}; - /// The object a `TSP.Reference` in field @p number names. std::optional reference_identifier(const Message &message, const std::uint32_t number) { @@ -88,103 +46,6 @@ std::vector reference_identifiers(const Message &message, return result; } -/// The character index each paragraph of @p storage starts at. Paragraph -/// boundaries are the run table's rather than every mark in the text — the two -/// agree today, but the table is what says so. -std::vector paragraph_starts(const Message &storage) { - std::vector result; - - if (const std::optional table = - storage.bytes_field(text_storage::paragraph_styles); - table.has_value()) { - for (const Field &entry : - Message(*table).repeated_field(attribute_table::entries)) { - if (entry.type != WireType::length_delimited) { - throw std::runtime_error("iwork: malformed paragraph style table"); - } - const Message run(entry.bytes); - result.push_back( - run.number_field(attribute_table_entry::character_index).value_or(0)); - } - } - - // no table, or an empty one: either way the body starts at its first - // character - if (result.empty() || result.front() != 0) { - result.insert(result.begin(), 0); - } - return result; -} - -/// Fills @p paragraph_id with the text of one paragraph, breaking it at the -/// line separators it holds. -void parse_paragraph(ElementRegistry ®istry, Budget &budget, - const ElementIdentifier paragraph_id, - std::string_view content) { - const auto append_text = [&](const std::string_view part) { - std::string text(part); - util::string::replace_all(text, std::string(object_replacement), ""); - if (text.empty()) { - return; - } - budget.spend_element(); - budget.spend_text(text.size()); - auto [text_id, element, payload] = registry.create_text_element(); - payload.text = std::move(text); - registry.append_child(paragraph_id, text_id); - }; - - for (std::size_t position = content.find(line_separator); - position != std::string_view::npos; - position = content.find(line_separator)) { - append_text(content.substr(0, position)); - - budget.spend_element(); - auto [break_id, element] = registry.create_element(ElementType::line_break); - registry.append_child(paragraph_id, break_id); - - content.remove_prefix(position + line_separator.size()); - } - append_text(content); -} - -/// Appends the paragraphs of a `TSWP.StorageArchive` to @p parent_id. Shared -/// by every place text lives: a Pages body and a Keynote text box. -void parse_storage(ElementRegistry ®istry, Budget &budget, - const ElementIdentifier parent_id, const Message &storage) { - // the text arrives as a small number of large strings; the run tables index - // it as one - std::string text; - for (const Field &part : storage.repeated_field(text_storage::text)) { - if (part.type != WireType::length_delimited) { - throw std::runtime_error("iwork: malformed text storage"); - } - text += part.bytes; - } - - const std::vector starts = - util::string::utf16_offsets(text, paragraph_starts(storage)); - - const std::string_view body_text(text); - for (std::size_t i = 0; i < starts.size(); ++i) { - const std::size_t begin = starts[i]; - const std::size_t end = i + 1 < starts.size() ? starts[i + 1] : text.size(); - - std::string_view content = body_text.substr(begin, end - begin); - // the paragraph mark belongs to the paragraph it ends, and the last - // paragraph of a body does not carry one - if (!content.empty() && is_paragraph_mark(content.back())) { - content.remove_suffix(1); - } - - budget.spend_element(); - auto [paragraph_id, paragraph] = - registry.create_element(ElementType::paragraph); - registry.append_child(parent_id, paragraph_id); - parse_paragraph(registry, budget, paragraph_id, content); - } -} - /// The root archive of the package's `Document` component, checked against the /// type the app is expected to write. const Object &root_archive(Package &package, const std::uint32_t type) { diff --git a/src/odr/internal/iwork/iwork_text.cpp b/src/odr/internal/iwork/iwork_text.cpp new file mode 100644 index 00000000..52422668 --- /dev/null +++ b/src/odr/internal/iwork/iwork_text.cpp @@ -0,0 +1,136 @@ +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace odr::internal::iwork { + +namespace { + +/// `U+2028 LINE SEPARATOR` — a line break inside a paragraph. +constexpr std::string_view line_separator = "\xe2\x80\xa8"; +/// `U+FFFC OBJECT REPLACEMENT CHARACTER` — where a drawable is anchored in the +/// text. Nothing reads drawables anchored in a text flow yet, so the anchor is +/// dropped rather than rendered as a glyph. +constexpr std::string_view object_replacement = "\xef\xbf\xbc"; + +/// The paragraph mark ends the paragraph it belongs to. Pages writes `\n` and +/// Keynote `\r` — the run table is what says where a paragraph starts either +/// way, so this only decides whether the mark is part of the text. +bool is_paragraph_mark(const char c) { return c == '\n' || c == '\r'; } + +/// The character index each paragraph of @p storage starts at. Paragraph +/// boundaries are the run table's rather than every mark in the text — the two +/// agree today, but the table is what says so. +std::vector paragraph_starts(const Message &storage) { + std::vector result; + + if (const std::optional table = + storage.bytes_field(text_storage::paragraph_styles); + table.has_value()) { + for (const Field &entry : + Message(*table).repeated_field(attribute_table::entries)) { + if (entry.type != WireType::length_delimited) { + throw std::runtime_error("iwork: malformed paragraph style table"); + } + const Message run(entry.bytes); + result.push_back( + run.number_field(attribute_table_entry::character_index).value_or(0)); + } + } + + // no table, or an empty one: either way the body starts at its first + // character + if (result.empty() || result.front() != 0) { + result.insert(result.begin(), 0); + } + return result; +} + +/// Fills @p paragraph_id with the text of one paragraph, breaking it at the +/// line separators it holds. +void parse_paragraph(ElementRegistry ®istry, Budget &budget, + const ElementIdentifier paragraph_id, + std::string_view content) { + const auto append_text = [&](const std::string_view part) { + std::string text(part); + util::string::replace_all(text, std::string(object_replacement), ""); + if (text.empty()) { + return; + } + budget.spend_element(); + budget.spend_text(text.size()); + auto [text_id, element, payload] = registry.create_text_element(); + payload.text = std::move(text); + registry.append_child(paragraph_id, text_id); + }; + + for (std::size_t position = content.find(line_separator); + position != std::string_view::npos; + position = content.find(line_separator)) { + append_text(content.substr(0, position)); + + budget.spend_element(); + auto [break_id, element] = registry.create_element(ElementType::line_break); + registry.append_child(paragraph_id, break_id); + + content.remove_prefix(position + line_separator.size()); + } + append_text(content); +} + +} // namespace + +} // namespace odr::internal::iwork + +namespace odr::internal { + +void iwork::parse_storage(ElementRegistry ®istry, Budget &budget, + const ElementIdentifier parent_id, + const Message &storage) { + // the text arrives as a small number of large strings; the run tables index + // it as one + std::string text; + for (const Field &part : storage.repeated_field(text_storage::text)) { + if (part.type != WireType::length_delimited) { + throw std::runtime_error("iwork: malformed text storage"); + } + text += part.bytes; + } + + const std::vector starts = + util::string::utf16_offsets(text, paragraph_starts(storage)); + + const std::string_view body_text(text); + for (std::size_t i = 0; i < starts.size(); ++i) { + const std::size_t begin = starts[i]; + const std::size_t end = i + 1 < starts.size() ? starts[i + 1] : text.size(); + + std::string_view content = body_text.substr(begin, end - begin); + // the paragraph mark belongs to the paragraph it ends, and the last + // paragraph of a body does not carry one + if (!content.empty() && is_paragraph_mark(content.back())) { + content.remove_suffix(1); + } + + budget.spend_element(); + auto [paragraph_id, paragraph] = + registry.create_element(ElementType::paragraph); + registry.append_child(parent_id, paragraph_id); + parse_paragraph(registry, budget, paragraph_id, content); + } +} + +} // namespace odr::internal diff --git a/src/odr/internal/iwork/iwork_text.hpp b/src/odr/internal/iwork/iwork_text.hpp new file mode 100644 index 00000000..d8d006c3 --- /dev/null +++ b/src/odr/internal/iwork/iwork_text.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include + +namespace odr::internal::iwork { +class Budget; +class ElementRegistry; +class Message; + +/// Appends the paragraphs of a `TSWP.StorageArchive` to @p parent_id. Shared +/// by every place text lives: a Pages body and a Keynote text box. +void parse_storage(ElementRegistry ®istry, Budget &budget, + ElementIdentifier parent_id, const Message &storage); + +} // namespace odr::internal::iwork