Pick the pages before rendering them, like OpenDocument.droid

OpenDocumentReader.xcodeproj/project.pbxproj

@@ -33,6 +33,8 @@
 		E22EB71C226B66B300053B86 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = E22EB71B226B66B300053B86 /* Main.storyboard */; };
 		E23795302274844400BA7238 /* AdSupport.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E237952F2274844400BA7238 /* AdSupport.framework */; };
 		E24110312586349500800247 /* test.odt in Resources */ = {isa = PBXBuildFile; fileRef = E24110232586349500800247 /* test.odt */; };
+		E2A17B1000000000000000B0 /* test.ods in Resources */ = {isa = PBXBuildFile; fileRef = E2A17B1200000000000000B2 /* test.ods */; };
+		E2A17B1100000000000000B1 /* test.odp in Resources */ = {isa = PBXBuildFile; fileRef = E2A17B1300000000000000B3 /* test.odp */; };
 		E26C39392250DC6E009C484A /* WebKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E26C39382250DC6E009C484A /* WebKit.framework */; };
 		E2C008FA220F1CF80097C594 /* CoreWrapper.mm in Sources */ = {isa = PBXBuildFile; fileRef = E2C008F9220F1CF80097C594 /* CoreWrapper.mm */; };
 		E2D0B3D9226D945400534FCC /* StoreReviewHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2D0B3D8226D945400534FCC /* StoreReviewHelper.swift */; };
@@ -98,6 +100,8 @@
 		E22EB71B226B66B300053B86 /* Main.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; path = Main.storyboard; sourceTree = "<group>"; };
 		E237952F2274844400BA7238 /* AdSupport.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = AdSupport.framework; path = System/Library/Frameworks/AdSupport.framework; sourceTree = SDKROOT; };
 		E24110232586349500800247 /* test.odt */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.odt; sourceTree = "<group>"; };
+		E2A17B1200000000000000B2 /* test.ods */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.ods; sourceTree = "<group>"; };
+		E2A17B1300000000000000B3 /* test.odp */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.odp; sourceTree = "<group>"; };
 		E26C39382250DC6E009C484A /* WebKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = WebKit.framework; path = System/Library/Frameworks/WebKit.framework; sourceTree = SDKROOT; };
 		E2BB4B60220EF3A10056176B /* BridgingHeader.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BridgingHeader.h; sourceTree = "<group>"; };
 		E2C008F9220F1CF80097C594 /* CoreWrapper.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CoreWrapper.mm; sourceTree = "<group>"; };
@@ -186,6 +190,8 @@
 			isa = PBXGroup;
 			children = (
 				E24110232586349500800247 /* test.odt */,
+				E2A17B1200000000000000B2 /* test.ods */,
+				E2A17B1300000000000000B3 /* test.odp */,
 				E22B252E2557F0E2001D0C52 /* OpenDocumentReaderTests.swift */,
 				E2A17B0300000000000000A3 /* PageTabBarTests.swift */,
 				E22B25302557F0E2001D0C52 /* Info.plist */,
@@ -356,6 +362,8 @@
 			buildActionMask = 2147483647;
 			files = (
 				E24110312586349500800247 /* test.odt in Resources */,
+				E2A17B1000000000000000B0 /* test.ods in Resources */,
+				E2A17B1100000000000000B1 /* test.odp in Resources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

OpenDocumentReader/CoreWrapper.mm

@@ -30,6 +30,41 @@
                            userInfo:@{NSLocalizedDescriptionKey: description}];
 }
 
+/// The view odrcore names "document" holds the whole file in one page: every
+/// slide of a presentation, every page of a PDF, the entire text document.
+static bool CoreWrapperIsCombinedView(const odr::HtmlView &view) {
+    return view.name() == "document";
+}
+
+/// Picks the views to show as pages, the same way OpenDocument.droid does.
+///
+/// Spreadsheets get one tab per sheet, because scrolling through every sheet of
+/// a workbook in one page is not how anyone reads a spreadsheet. Everything else
+/// gets the combined view and nothing else - a presentation would otherwise show
+/// its slides twice, once inside the combined view and once per slide view, and
+/// a PDF would list one tab per page next to the tab that already has them all.
+///
+/// Services without a combined view - plain text and images among them - keep
+/// whatever views they do offer.
+static odr::HtmlViews CoreWrapperSelectViews(const odr::HtmlViews &views,
+                                             odr::DocumentType documentType) {
+    bool isSpreadsheet = documentType == odr::DocumentType::spreadsheet;
+    bool hasCombinedView = std::any_of(views.begin(), views.end(), CoreWrapperIsCombinedView);
+
+    odr::HtmlViews selected;
+    for (const odr::HtmlView &view : views) {
+        bool isCombinedView = CoreWrapperIsCombinedView(view);
+        bool skip = isSpreadsheet ? isCombinedView : (hasCombinedView && !isCombinedView);
+        if (skip) {
+            continue;
+        }
+
+        selected.push_back(view);
+    }
+
+    return selected;
+}
+
 /// odrcore's data files ship inside the app bundle. The path never changes at
 /// runtime, so it is set once instead of on every translate call.
 static void CoreWrapperEnsureDataPath() {
@@ -119,27 +154,29 @@ - (BOOL)translate:(NSString *)inputPath
             odr::DocumentType documentType = documentFile.document_type();
             _document = documentFile.document();
 
-            _html = odr::html::translate(*_document, cachePathCpp, config).bring_offline(outputPathCpp);
-
-            NSMutableArray<NSString *> *pageNames = [[NSMutableArray alloc] init];
-            NSMutableArray<NSString *> *pagePaths = [[NSMutableArray alloc] init];
-            for (const auto &page : _html->pages()) {
-                if ([self shouldSkipPageNamed:page.name ofDocumentType:documentType]) {
-                    continue;
-                }
-
-                [pageNames addObject:[NSString stringWithUTF8String:page.name.c_str()]];
-                [pagePaths addObject:[NSString stringWithUTF8String:page.path.c_str()]];
-            }
+            odr::HtmlService service = odr::html::translate(*_document, cachePathCpp, config);
 
-            if (pagePaths.count == 0) {
+            // the views are picked before they are rendered: bringing all of them
+            // offline first would write out every slide of a presentation only to
+            // throw the files away again
+            odr::HtmlViews views = CoreWrapperSelectViews(service.list_views(), documentType);
+            if (views.empty()) {
                 if (error) {
                     *error = CoreWrapperMakeError(CoreWrapperErrorUnknown,
                                                   @"odrcore produced no displayable page");
                 }
                 return NO;
             }
 
+            _html = service.bring_offline(outputPathCpp, views);
+
+            NSMutableArray<NSString *> *pageNames = [[NSMutableArray alloc] init];
+            NSMutableArray<NSString *> *pagePaths = [[NSMutableArray alloc] init];
+            for (const auto &page : _html->pages()) {
+                [pageNames addObject:[NSString stringWithUTF8String:page.name.c_str()]];
+                [pagePaths addObject:[NSString stringWithUTF8String:page.path.c_str()]];
+            }
+
             _pageNames = pageNames;
             _pagePaths = pagePaths;
 
@@ -165,21 +202,6 @@ - (BOOL)translate:(NSString *)inputPath
     }
 }
 
-/// Presentations and drawings expose their slides as pages and a combined
-/// "document" page we do not want; spreadsheets are the other way round.
-- (BOOL)shouldSkipPageNamed:(const std::string &)name ofDocumentType:(odr::DocumentType)documentType {
-    bool isCombinedPage = name == "document";
-
-    if (documentType == odr::DocumentType::presentation ||
-        documentType == odr::DocumentType::drawing) {
-        return isCombinedPage ? NO : YES;
-    }
-    if (documentType == odr::DocumentType::spreadsheet) {
-        return isCombinedPage ? YES : NO;
-    }
-    return NO;
-}
-
 - (BOOL)backTranslate:(NSString *)diff into:(NSString *)outputPath error:(NSError **)error {
     @synchronized(self) {
         if (!_document.has_value()) {

OpenDocumentReaderTests/OpenDocumentReaderTests.swift

@@ -14,21 +14,29 @@ class OpenDocumentReaderTests: XCTestCase {
     private var saveURL: URL!
 
     override func setUpWithError() throws {
+        saveURL = try copyFixture(ofType: "odt")
+    }
+
+    /// Copies a bundled fixture next to the documents directory, because
+    /// translating writes its cache and output beside the input.
+    private func copyFixture(ofType pathExtension: String) throws -> URL {
         let documentsURL = try FileManager.default.url(
             for: .documentDirectory,
             in: .userDomainMask,
             appropriateFor: nil,
             create: false)
 
-        saveURL = documentsURL.appendingPathComponent("test.odt")
+        let url = documentsURL.appendingPathComponent("test." + pathExtension)
 
-        if FileManager.default.fileExists(atPath: saveURL.path) {
-            try FileManager.default.removeItem(at: saveURL)
+        if FileManager.default.fileExists(atPath: url.path) {
+            try FileManager.default.removeItem(at: url)
         }
 
         let bundlePath = try XCTUnwrap(
-            Bundle(for: type(of: self)).path(forResource: "test", ofType: "odt"))
-        try FileManager.default.copyItem(at: URL(fileURLWithPath: bundlePath), to: saveURL)
+            Bundle(for: Self.self).path(forResource: "test", ofType: pathExtension))
+        try FileManager.default.copyItem(at: URL(fileURLWithPath: bundlePath), to: url)
+
+        return url
     }
 
     private func makeWrapper() -> (wrapper: CoreWrapper, cache: String, output: String) {
@@ -49,6 +57,51 @@ class OpenDocumentReaderTests: XCTestCase {
         }
     }
 
+    /// A text document has nothing but its combined view.
+    func testTextDocumentIsASinglePage() throws {
+        let (wrapper, cache, output) = makeWrapper()
+
+        try wrapper.translate(saveURL.path, cache: cache, into: output, with: nil, editable: false)
+
+        XCTAssertEqual(wrapper.pageNames, ["document"])
+    }
+
+    /// Spreadsheets are the one format that drops the combined view: a tab per
+    /// sheet is how a workbook is read.
+    func testSpreadsheetBecomesOnePagePerSheet() throws {
+        let (wrapper, cache, output) = makeWrapper()
+        let url = try copyFixture(ofType: "ods")
+
+        try wrapper.translate(url.path, cache: cache, into: output, with: nil, editable: false)
+
+        XCTAssertEqual(wrapper.pageNames, ["Alpha", "Beta", "Gamma"])
+    }
+
+    /// The combined view of a presentation already holds every slide, so
+    /// listing the slides next to it would show each of them twice.
+    func testPresentationKeepsOnlyTheCombinedPage() throws {
+        let (wrapper, cache, output) = makeWrapper()
+        let url = try copyFixture(ofType: "odp")
+
+        try wrapper.translate(url.path, cache: cache, into: output, with: nil, editable: false)
+
+        XCTAssertEqual(wrapper.pageNames, ["document"])
+    }
+
+    /// Views that are not shown must not be rendered either.
+    func testDiscardedPagesAreNotWrittenOut() throws {
+        let (wrapper, cache, _) = makeWrapper()
+        let output = NSTemporaryDirectory() + "presentation-output"
+        try? FileManager.default.removeItem(atPath: output)
+        let url = try copyFixture(ofType: "odp")
+
+        try wrapper.translate(url.path, cache: cache, into: output, with: nil, editable: false)
+
+        let written = try FileManager.default.contentsOfDirectory(atPath: output)
+            .filter { $0.hasSuffix(".html") }
+        XCTAssertEqual(written, ["document.html"])
+    }
+
     func testBackTranslateWritesEditedDocument() throws {
         let (wrapper, cache, output) = makeWrapper()
 

OpenDocumentReaderTests/fixtures/make-fixtures.py

@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+"""Builds the minimal ODF packages the page selection tests translate.
+
+The sample documents we have lying around are megabytes of third party
+material; these are a few hundred bytes each and exist only so a test can ask
+"how many pages does a two sheet spreadsheet turn into". Run this from the
+repository root when a fixture needs another sheet or slide:
+
+    python3 OpenDocumentReaderTests/fixtures/make-fixtures.py
+"""
+
+import zipfile
+from pathlib import Path
+
+NAMESPACES = " ".join(
+    [
+        'xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0"',
+        'xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0"',
+        'xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"',
+        'xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0"',
+        'xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0"',
+        'xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0"',
+        'xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0"',
+    ]
+)
+
+MANIFEST = """<?xml version="1.0" encoding="UTF-8"?>
+<manifest:manifest xmlns:manifest="urn:oasis:names:tc:opendocument:xmlns:manifest:1.0" manifest:version="1.2">
+ <manifest:file-entry manifest:full-path="/" manifest:media-type="{mimetype}"/>
+ <manifest:file-entry manifest:full-path="content.xml" manifest:media-type="text/xml"/>
+ <manifest:file-entry manifest:full-path="styles.xml" manifest:media-type="text/xml"/>
+</manifest:manifest>
+"""
+
+STYLES = f"""<?xml version="1.0" encoding="UTF-8"?>
+<office:document-styles {NAMESPACES} office:version="1.2">
+ <office:styles/>
+ <office:automatic-styles>
+  <style:page-layout style:name="PM1">
+   <style:page-layout-properties fo:page-width="28cm" fo:page-height="21cm"/>
+  </style:page-layout>
+ </office:automatic-styles>
+ <office:master-styles>
+  <style:master-page style:name="Default" style:page-layout-name="PM1"/>
+ </office:master-styles>
+</office:document-styles>
+"""
+
+
+def content(body: str) -> str:
+    return f"""<?xml version="1.0" encoding="UTF-8"?>
+<office:document-content {NAMESPACES} office:version="1.2">
+ <office:body>
+{body}
+ </office:body>
+</office:document-content>
+"""
+
+
+def spreadsheet(sheets: list[str]) -> str:
+    tables = "\n".join(
+        f"""   <table:table table:name="{sheet}">
+    <table:table-column/>
+    <table:table-row>
+     <table:table-cell office:value-type="string"><text:p>{sheet} A1</text:p></table:table-cell>
+    </table:table-row>
+   </table:table>"""
+        for sheet in sheets
+    )
+    return content(f"  <office:spreadsheet>\n{tables}\n  </office:spreadsheet>")
+
+
+def presentation(slides: list[str]) -> str:
+    pages = "\n".join(
+        f"""   <draw:page draw:name="{slide}" draw:master-page-name="Default">
+    <draw:frame svg:width="10cm" svg:height="2cm" svg:x="1cm" svg:y="1cm">
+     <draw:text-box><text:p>{slide}</text:p></draw:text-box>
+    </draw:frame>
+   </draw:page>"""
+        for slide in slides
+    )
+    return content(f"  <office:presentation>\n{pages}\n  </office:presentation>")
+
+
+def write(path: Path, mimetype: str, content_xml: str) -> None:
+    with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as package:
+        # the mimetype entry has to come first and stay uncompressed for the
+        # package to be recognised without sniffing the contents
+        package.writestr(
+            zipfile.ZipInfo("mimetype"), mimetype, compress_type=zipfile.ZIP_STORED
+        )
+        package.writestr("META-INF/manifest.xml", MANIFEST.format(mimetype=mimetype))
+        package.writestr("styles.xml", STYLES)
+        package.writestr("content.xml", content_xml)
+    print(f"wrote {path}")
+
+
+def main() -> None:
+    here = Path(__file__).parent
+
+    write(
+        here.parent / "test.ods",
+        "application/vnd.oasis.opendocument.spreadsheet",
+        spreadsheet(["Alpha", "Beta", "Gamma"]),
+    )
+    write(
+        here.parent / "test.odp",
+        "application/vnd.oasis.opendocument.presentation",
+        presentation(["Intro", "Outro"]),
+    )
+
+
+if __name__ == "__main__":
+    main()