From b84574e6de64721f729e6d46fb46182d921d0299 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 11:53:21 +0200 Subject: [PATCH] Pick the pages before rendering them, like OpenDocument.droid Which odrcore views become pages was decided per document type: keep the combined "document" view for presentations and drawings, drop it for spreadsheets, keep everything for anything else. OpenDocument.droid asks a different question - is there a combined view at all? - and that one also holds for the formats iOS did not enumerate. A PDF translated by core has a combined view plus one view per page, so the old rule would have offered a tab per page next to the tab that already has them all. PDFs still go to WKWebView, so this is not visible yet; the point is that the rule no longer has to be extended when they stop doing so. The selection also moved in front of the rendering. `bring_offline()` wrote out every view and the discarded ones were dropped afterwards, so a fifty slide deck wrote fifty one HTML files to show one of them. `bring_offline(path, views)` renders only what is going to be shown. The two fixtures are minimal ODF packages built by the script next to them, a few hundred bytes each, rather than samples from OpenDocument.test that are megabytes of third party material. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5 --- OpenDocumentReader.xcodeproj/project.pbxproj | 8 ++ OpenDocumentReader/CoreWrapper.mm | 78 +++++++----- .../OpenDocumentReaderTests.swift | 63 +++++++++- .../fixtures/make-fixtures.py | 114 ++++++++++++++++++ OpenDocumentReaderTests/test.odp | Bin 0 -> 1292 bytes OpenDocumentReaderTests/test.ods | Bin 0 -> 1272 bytes 6 files changed, 230 insertions(+), 33 deletions(-) create mode 100644 OpenDocumentReaderTests/fixtures/make-fixtures.py create mode 100644 OpenDocumentReaderTests/test.odp create mode 100644 OpenDocumentReaderTests/test.ods 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 0000000000000000000000000000000000000000..75cbc57922e2051fa66d8fa0189a0009bde0f524 GIT binary patch literal 1292 zcmWIWW@Zs#fB;1XW3@Fa^nn}@<^bZ{%-qzH%7WCyf`XjP6GBAh&Xq{@8UseC5%iFzqzDlZp>*RTJi+{z$7%7`t=^s|Iv;8f;go^9_6pZ|`cI9c= zkF_QS{!1pOo}ZQDa_v#cDG%qOVi%#m;p@Yf|K7s6p?%*f`MSmYH>Ui~tXy&2vPCWW zr{My#2_4PbdvzpvZ_H^f5911%o0)Mv<*ix3Z1pEW;k+x#ePWe%3Z0r9cY{05CH8uL zVdS)iKmIw(=lxrCwHV~v-r$3IEeaxgev7tP zZ>_o|P^v%s-mEVd>KI%LJXv}cy#80~SaLdRcEwzC<9xaIfpgSvHckBMmJxdB%~MgP zWh>UTW_kVi`m@dNLyT(cvzUOkY+v@1Q(TYTi%x40)^2v**y6l?gPNPfnvN8<=&sV^ zSF{cpN}pfLWHeiL>iH)Txi?e#yKKLEdhhEmpLcTOu6HNjuDDTrclNQOb^TrHmzCBD zUfrY|mJ>8%qvfBnaC5bCk%WWeB z1m-Vmk$*q)`=9RiUH4eut}VEJq;$m{m)!NLXZ%ABu~(!n+PTa1`Ap}_J2Q6`e>S}Q zzj&RpN0QF#??1)wNZUQ#Z{XK<&1pmbf9nJ(^ZO+g4`%a+sM@0j=S{1=k8Z%=3}azn z;06X~a(-S3DCGge^K8iOycPq2H$S;f+deD`O_5yJskGEl=F; zjn64WdElZ=t<<+jx~93KVLJ`2ANU6)L*omlx|gTG14mEOA_D}8;o?V9(zfG?H*%2Od`y< zO9Nn-gFyo@5)fqrLKpVz4$;HF(7+f9WMa+y=tiJt9E1^mK&uhi2wfj~3PI?b%Y>3% V0=!w-K$0v#_zFn#0Fx900|4#X13Ul# literal 0 HcmV?d00001 diff --git a/OpenDocumentReaderTests/test.ods b/OpenDocumentReaderTests/test.ods new file mode 100644 index 0000000000000000000000000000000000000000..08f76e4b1222adeeaedf5525400cc31b14876be0 GIT binary patch literal 1272 zcmWIWW@Zs#fB;2?)*Q<&Js<~!Ie<7fGdH!QvLH3FpdcqRIk6-&KTp3bFGVjuu{g6> zFTWr)FC{;@G&eP`M6bA@C^a#qI3qQ+BminA2Saw;ub4@u^J0$y%{s@(z#s~weO*Hw zbv^yu^m7yQGSgCvOY|yobEZyo>^f|~#CVG)!x_gn778bZJBqzgXxv5|8&oF4PO_jziSc? zy#6sdW}2$f{*Jd-_D}zEl^@mP6C)G))q$RFWM*LC0(!i-q%tS9806dD;DdQB3L<-c zi?&#At-2*psz3YQtS=Yp7+ecHS$Y<{{#WZ*ayn~v#awgae7X05bJTA(P5kPX5qjv& zQ&Fa6E7rAUdHwkMv(4{AjB4w%n1HryU-pwzT#wz0PHPa>Zg$?-;=F!?nw!L$juf`& zuF~UIvjWwD`y~|*X7h)r+M@<%_>cZo#=zhd zVP#<81_ozxeqISE;Q_;QZ-{qZvw_H+Vy@HGUYA2tB^RY!j9Xyaz|7zDg#S?6fw$Q? zO%02>T#vH-Ic7ZX`$nJhb04f|jhgOt?8lc+U4kb#z zQ?}x;XTv<@{VUgOe<<>L72C9z)(w&gF16uz#D9LB;Mk+TXP5CC3+_q24dza6(!hjL zQKHIlZn=qMzJf_@s_d4Px$cEdcSBYmv0QYz(@i-+e08$h<&4v%jsIeAi=NrIl((%t zi)|*)fAnv?nDKd;--i7cf_Kzin)HA@z?+dtgc*0v2L>=0Gyp>!Q4Sz