Let the core render the raw files it has learned to render - #564
Merged
Conversation
odrcore 6.2 turns text, images, zip and cfb, fonts, audio and video into html itself, so the viewers RawLoader kept in assets/ - PhotoSwipe, Plyr, JSZip - had nothing left to do. They were in fact already unreachable: LoaderService routes METADATA -> CORE unconditionally and only falls back to RawLoader when the core *throws*, which it stopped doing for those formats. Deleting them takes 916KB of vendored javascript with them. What is left in RawLoader has a reason to be: - csv, because the core files it as text and renders it line by line where text-prefix.html builds a real table. This one is a restoration: on main a .csv already showed as plain text, the table viewer having quietly died with the 6.2 upgrade. It is excluded from CORE_FILE_TYPES and asked for first, since the core would otherwise succeed and RawLoader would never get a turn. - svg, which odrcore has no file type for at all. - xml and whatever else reaches the generic branch. SupportedDocumentTypes now answers two questions instead of one. What the app *claims* (CLAIMED_FILE_TYPES) is unchanged, so AndroidManifest.xml does not move and the app still stays out of the share sheet for an mp3; what CoreLoader *renders* (CORE_FILE_TYPES) is now every translateHtml format whatever its category, so an mp3 handed to us still plays. Two bugs fixed on the way: - OnlineLoader used CoreLoader.isSupported to mean "is an office document", which stops being true once the core loader claims images and media - a png would have been sent to the libreoffice converter and the microsoft viewer. It asks SupportedDocumentTypes.isDocument now, which also makes .xlsb convertible: the converter is not odrcore and can manage one. - svg never reached RawLoader at all. MetadataLoader asks Odr.mimetype first and the core content-detects an svg as text/plain, so the image/svg+xml a provider volunteered never reached routing - and MimeTypeResolver.resolve then rewrites the extension to "txt", losing the name too. Routing on the original filename fixes it, and an svg renders as an image for the first time rather than as a line-numbered text dump. The "open it in another app" hint follows the core's file category now rather than "the raw loader ran", which said the same thing only while everything that was not a document went to RawLoader. RawLoaderTest moves to androidTest: the csv spellings come from the core's table, which lives in libodr_jni. Verified on an emulator against a build of main over the same corpus: png, wav, txt, zip and odt route identically, and only csv, svg and xml change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhqfvUDxKNH7b9ueC5iuE
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20c2e05e0c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Routing svg and xml by their filename, which the previous commit added because the core content-detects both as text/plain, was too eager: it also took a file the core *had* identified. An odt called report.csv went to RawLoader on the strength of its name, which base64'd the zip into the table viewer and reported success - and success leaves no fallback, so the document was simply gone. The name only gets a say now where the detection has none to give: the core recognized nothing, or recognized plain text. Anything it identified as a format of its own keeps that identity. Reported by codex on #564, reproduced on an emulator (report.csv holding an odt opened blank, now renders as a document again) and covered by a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhqfvUDxKNH7b9ueC5iuE
Cut the KDoc and inline comments added by this branch to roughly half. The originals restated what the code said next to them; what is left is the part a reader could not reconstruct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhqfvUDxKNH7b9ueC5iuE
andiwand
added a commit
that referenced
this pull request
Aug 3, 2026
The store listing has carried the same three release notes since 2024 - 137.txt, 93.txt and 82.txt all say "Fix potential crash while saving changes for a document" - so every release since has gone out with whatever the console last had, and there is nowhere in the tree that says what a user got. One file per release under fastlane/metadata, named by version code, which is what `fastlane supply` uploads as the Play Console release notes. The codes jump: 4.6, 4.7 and 4.8.0 were counted by hand (202-204) and everything from 4.9.0 on is derived from the tag (40900 and up). All of them are inside Play's 500 character limit. CHANGELOG.md is the same history at more length, for the repo and for github releases. Three of the eight releases say "maintenance release, no visible changes", because that is what they were: 4.6 was CI cleanup and a submodule bump of renovate commits, 4.8.0 was the release workflow and the instrumented test flakiness, and 4.12.0 was moving odrcore from a conan build to the maven central AAR. Core changes are folded into the release that absorbed them rather than listed on their own, since a user has no way to tell where a change came from. That puts almost everything in 4.7, which took odrcore 5.2.0 -> 5.7.1 in one go: the whole PDF renderer, the built-in doc/xls/ppt implementations, and the drop of the pdf2htmlEX and wvWare backends. Nothing about formats that disappeared and came back - the csv table viewer that died with the 6.2 upgrade and returned in #564 reads only as "text, images, archives and media are shown by the engine", and the lite review prompt that had been dead since 4.2 is not described as broken. Play-facing wording says "our own engine" rather than naming odrcore, which store readers have no reason to know. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
RawLoaderwas written when odrcore could only turn office documents into html. Everything else — text, csv, images, audio, video, zip — got a viewer of its own out ofassets/: PhotoSwipe, Plyr, JSZip, csv-to-html-table.odrcore 6.2, which we already ship, renders all of them. Verified against
OpenDocument.coreat thev6.2.0tag:translate_htmlis set for text, csv, zip and cfb, nine image formats (webp, tiff, heic and avif joined the original five), fonts, and all five audio and five video types.html.cppdispatches them to a line-numbered text view, an<img>page, an archive listing and a real<audio controls>/<video controls playsinline>player.Most of this was already dead code
LoaderService.onSuccessroutes METADATA → CORE unconditionally andFileLoader.loadAsynchas noisSupportedguard —isSupportedonly informs routing. RawLoader was reached from exactly one place:LoaderService.onError, when the core threw. Since the core stopped throwing for those formats, those branches stopped running.Confirmed empirically rather than by reading: I built
mainin a scratch worktree and ran the same corpus through both. Onmaina.csvalready renders as the core's plain text — the table viewer had quietly died with the 6.2 upgrade.Deleting the four assets removes 916KB of vendored javascript.
What stays
text-prefix.htmlbuilds a real table. Bringing it back is a deliberate feature change, not preservation. It is excluded fromCORE_FILE_TYPESand asked for before the core, which otherwise succeeds and RawLoader never gets a turn.Claim vs render
SupportedDocumentTypesanswers two questions now. What the app claims (CLAIMED_FILE_TYPES) is unchanged, soAndroidManifest.xmldoes not move and we stay out of the share sheet for an mp3. WhatCoreLoaderrenders (CORE_FILE_TYPES) is now everytranslateHtmlformat whatever its category — so an mp3 handed to us still plays.Two bugs fixed on the way
OnlineLoaderusedCoreLoader.isSupportedto mean "is an office document". That stops being true the moment the core loader claims images and media, so a png would have gone to the LibreOffice converter and the Microsoft viewer. It asksSupportedDocumentTypes.isDocumentnow — which also makes.xlsbconvertible, correctly: the converter is not odrcore.MetadataLoaderasksOdr.mimetypefirst and the core content-detects an svg astext/plain, so theimage/svg+xmla provider volunteered never reached routing — andMimeTypeResolver.resolvethen rewrites the extension totxt, losing the name too. Routing on the original filename fixes it. An svg renders as an image for the first time, rather than as a line-numbered text dump.The "open it in another app" hint follows the core's file category now rather than "the raw loader ran" — which meant the same thing only while everything non-document went to RawLoader.
RawLoaderTestmoves toandroidTest: the csv spellings come from a table inlibodr_jni.Verification
spotlessCheck,lintProDebug,testProDebugUnitTestand 55 instrumented tests pass on a Pixel 6 Pro emulator.Same corpus through a build of
mainand this branch, comparing which loader and which view each file got:image.htmlaudio.htmlScreenshots confirm the csv table and the svg drawing.
One judgement call worth a look
A failed RawLoader load now falls through to the core rather than erroring —
DocumentFragment.onUnsupportedhas no RAW branch and would otherwise show a blank screen. Easy to change if you would rather it report the error.🤖 Generated with Claude Code