diff --git a/OpenDocumentReader.xcodeproj/project.pbxproj b/OpenDocumentReader.xcodeproj/project.pbxproj index 227e50c..313e50e 100644 --- a/OpenDocumentReader.xcodeproj/project.pbxproj +++ b/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 = ""; }; 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 = ""; }; + E2A17B1200000000000000B2 /* test.ods */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.ods; sourceTree = ""; }; + E2A17B1300000000000000B3 /* test.odp */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.odp; sourceTree = ""; }; 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 = ""; }; E2C008F9220F1CF80097C594 /* CoreWrapper.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CoreWrapper.mm; sourceTree = ""; }; @@ -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; }; diff --git a/OpenDocumentReader/CoreWrapper.mm b/OpenDocumentReader/CoreWrapper.mm index 3c8923f..12aa81a 100644 --- a/OpenDocumentReader/CoreWrapper.mm +++ b/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,20 +154,13 @@ - (BOOL)translate:(NSString *)inputPath odr::DocumentType documentType = documentFile.document_type(); _document = documentFile.document(); - _html = odr::html::translate(*_document, cachePathCpp, config).bring_offline(outputPathCpp); - - NSMutableArray *pageNames = [[NSMutableArray alloc] init]; - NSMutableArray *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"); @@ -140,6 +168,15 @@ - (BOOL)translate:(NSString *)inputPath return NO; } + _html = service.bring_offline(outputPathCpp, views); + + NSMutableArray *pageNames = [[NSMutableArray alloc] init]; + NSMutableArray *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()) { diff --git a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift index 104fa9f..772ffeb 100644 --- a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift +++ b/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() diff --git a/OpenDocumentReaderTests/fixtures/make-fixtures.py b/OpenDocumentReaderTests/fixtures/make-fixtures.py new file mode 100644 index 0000000..c0edaa6 --- /dev/null +++ b/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 = """ + + + + + +""" + +STYLES = f""" + + + + + + + + + + + +""" + + +def content(body: str) -> str: + return f""" + + +{body} + + +""" + + +def spreadsheet(sheets: list[str]) -> str: + tables = "\n".join( + f""" + + + {sheet} A1 + + """ + for sheet in sheets + ) + return content(f" \n{tables}\n ") + + +def presentation(slides: list[str]) -> str: + pages = "\n".join( + f""" + + {slide} + + """ + for slide in slides + ) + return content(f" \n{pages}\n ") + + +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() diff --git a/OpenDocumentReaderTests/test.odp b/OpenDocumentReaderTests/test.odp new file mode 100644 index 0000000..75cbc57 Binary files /dev/null and b/OpenDocumentReaderTests/test.odp differ diff --git a/OpenDocumentReaderTests/test.ods b/OpenDocumentReaderTests/test.ods new file mode 100644 index 0000000..08f76e4 Binary files /dev/null and b/OpenDocumentReaderTests/test.ods differ