Pick the pages before rendering them, like OpenDocument.droid - #124
Merged
Conversation
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Why
CoreWrapperdecided which odrcore views become pages per document type: keep the combineddocumentview for presentations and drawings, drop it for spreadsheets, keep everything for anything else.OpenDocument.droid asks a different question — does the service have a combined view at all? — and that rule also holds for the formats iOS never enumerated. A PDF translated by core has a combined view plus one view per page, so the old rule would have shown a tab per page right next to the tab that already contains all of them. PDFs still go to WKWebView, so nothing about them changes here; the point is that the rule no longer needs another
ifwhen they stop doing so.What changed
CoreWrapperSelectViewsreplacesshouldSkipPageNamed:ofDocumentType:— spreadsheets drop the combined view, everything else keeps only the combined view when there is one, and services without one (plain text, images) keep whatever they offer. Same rule ascore_wrapper.cppon Android.bring_offline()wrote every view to disk and the discarded ones were thrown away afterwards, so a 50 slide deck wrote 51 HTML files to display one.bring_offline(path, views)renders only what will be shown.Tests
Four new cases in
OpenDocumentReaderTests: a text document is one page, a spreadsheet is one page per sheet, a presentation is only the combined page, and — the one that fails onmain— the slides of a presentation are no longer written to the output directory.The two fixtures are minimal ODF packages generated by
OpenDocumentReaderTests/fixtures/make-fixtures.py(~1 KB each), rather than samples from OpenDocument.test that are megabytes of third party material.Verified locally:
xcodebuild teston the iPhone 17 simulator, all 24 tests pass, plusscripts/format.sh.