Open every format the core renders, not just half of them - #546
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939d7a8baf
ℹ️ 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".
odrcore v6 keeps reference html output for doc, ppt, xls and pptx next to the ones the app already claimed, so the "TODO: enable pptx too" that sat in CoreLoader was the only thing keeping presentations out - and the legacy binary formats reached the core only by accident, through the doc mime type that was listed before ppt and xls ever were. What the loader claims is now one list: opendocument, the whole ooxml family, the three legacy microsoft formats and pdf. Prefixes, so the templates and the macro enabled variants come along - the latter are spelled vnd.ms-word/vnd.ms-excel/vnd.ms-powerpoint rather than under the openxmlformats prefix, which is why those appear without their subtypes. Text, csv and images stay out: the core renders them too, but RawLoader is what gives them their player, and it only gets a turn when the core loader says no. The manifest filter and SupportedDocumentTypes follow, so the system offers us pptx/ppt/xls and the folder browser lists them. Two things fall out of it. The doOoxml flag is gone - it has been passed true since ooxml stopped being experimental and now gates nothing. And the ".doc-files are not real documents" special case becomes a check for all three legacy types: the core does parse them now, but declares them neither editable nor savable, so keeping one open only buys a second parse. pdf changes which viewer a failed upload falls back to, so that stays explicit: google docs still gets the pdf, the microsoft viewer keeps everything else the core knows. Verified on device - .pptx, .ppt, .doc and .xls all render through the core, with CoreTest covering each of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp
andiwand
force-pushed
the
core/supported-formats
branch
from
July 28, 2026 19:08
939d7a8 to
424c4c9
Compare
Codex was right on #546: doc, ppt and xls now reach CoreLoader, and prepareActions offered Edit for every CORE result except ODS and PDF - so the legacy formats got a button that could only ever end in a failed save, because host() would not keep a document open for them. The list of exceptions was never ours to keep. odrcore answers this itself: Document.isEditable()/isSavable() is false for the three legacy binary formats, for ooxml spreadsheets and presentations, and for every spreadsheet including odf - the last being the core's own TODO, the same gap as #442 that the UI was spelling out by mime prefix. host() now keeps the document only when the core says yes, which makes "we have a document" the answer, and it travels to the fragment on Result. While in there, two more copies of the same idea: - CoreLoader.MIME_PREFIXES was a second SupportedDocumentTypes. It is gone; the loader defers to the table. What stays separate is the manifest, which XML keeps out of reach - so SupportedFormatsTest walks odrcore's FileType values, asks Odr.mimetypeByFileType for each, and asserts the table and the package manager agree. Adding a format to one side and forgetting the other is now a red test. The core cannot replace the table itself: its lookups are native, and know one canonical mime per format - no templates, no macroEnabled, no x-vnd.oasis, which is exactly what providers hand out. - toggleDarkMode's isDarkModeSupported was dead. Nothing in the body read it while DocumentFragment worked out that a pdf is not worth inverting. Renamed to disableDarkening(), which is what it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp
andiwand
added a commit
that referenced
this pull request
Jul 28, 2026
* Open every format the core renders, not just half of them odrcore v6 keeps reference html output for doc, ppt, xls and pptx next to the ones the app already claimed, so the "TODO: enable pptx too" that sat in CoreLoader was the only thing keeping presentations out - and the legacy binary formats reached the core only by accident, through the doc mime type that was listed before ppt and xls ever were. What the loader claims is now one list: opendocument, the whole ooxml family, the three legacy microsoft formats and pdf. Prefixes, so the templates and the macro enabled variants come along - the latter are spelled vnd.ms-word/vnd.ms-excel/vnd.ms-powerpoint rather than under the openxmlformats prefix, which is why those appear without their subtypes. Text, csv and images stay out: the core renders them too, but RawLoader is what gives them their player, and it only gets a turn when the core loader says no. The manifest filter and SupportedDocumentTypes follow, so the system offers us pptx/ppt/xls and the folder browser lists them. Two things fall out of it. The doOoxml flag is gone - it has been passed true since ooxml stopped being experimental and now gates nothing. And the ".doc-files are not real documents" special case becomes a check for all three legacy types: the core does parse them now, but declares them neither editable nor savable, so keeping one open only buys a second parse. pdf changes which viewer a failed upload falls back to, so that stays explicit: google docs still gets the pdf, the microsoft viewer keeps everything else the core knows. Verified on device - .pptx, .ppt, .doc and .xls all render through the core, with CoreTest covering each of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Ask the core what can be edited instead of guessing by mime type Codex was right on #546: doc, ppt and xls now reach CoreLoader, and prepareActions offered Edit for every CORE result except ODS and PDF - so the legacy formats got a button that could only ever end in a failed save, because host() would not keep a document open for them. The list of exceptions was never ours to keep. odrcore answers this itself: Document.isEditable()/isSavable() is false for the three legacy binary formats, for ooxml spreadsheets and presentations, and for every spreadsheet including odf - the last being the core's own TODO, the same gap as #442 that the UI was spelling out by mime prefix. host() now keeps the document only when the core says yes, which makes "we have a document" the answer, and it travels to the fragment on Result. While in there, two more copies of the same idea: - CoreLoader.MIME_PREFIXES was a second SupportedDocumentTypes. It is gone; the loader defers to the table. What stays separate is the manifest, which XML keeps out of reach - so SupportedFormatsTest walks odrcore's FileType values, asks Odr.mimetypeByFileType for each, and asserts the table and the package manager agree. Adding a format to one side and forgetting the other is now a red test. The core cannot replace the table itself: its lookups are native, and know one canonical mime per format - no templates, no macroEnabled, no x-vnd.oasis, which is exactly what providers hand out. - toggleDarkMode's isDarkModeSupported was dead. Nothing in the body read it while DocumentFragment worked out that a pdf is not worth inverting. Renamed to disableDarkening(), which is what it does. 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
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.
odrcore v6 keeps reference html output for
doc,ppt,xlsandpptxnext to the formats the app already claimed, so theTODO: enable pptx tooinCoreLoaderwas the only thing keeping presentations out. The legacy binary formats reached the core only by accident, through theapplication/mswordentry that was listed beforevnd.ms-powerpointandvnd.ms-excelever were.What the loader claims is now one list: opendocument, the whole ooxml family, the three legacy microsoft formats and pdf. Prefixes, so the templates and the macro enabled variants come along — the latter are spelled
vnd.ms-word/vnd.ms-excel/vnd.ms-powerpointrather than under the openxmlformats prefix, which is why those appear without their subtypes.Text, csv and images stay out: the core renders them too, but
RawLoaderis what gives them their player, and it only gets a turn whenCoreLoader.isSupported()says no.The
STRICT_CATCHfilter andSupportedDocumentTypesfollow, so the system offers us pptx/ppt/xls and the folder browser lists them.Two things fall out of it:
doOoxmlis gone. It has been passedtruesince ooxml stopped being experimental and now gates nothing..doc-files are not real documentsspecial case becomes a check for all three legacy types. The core does parse them now, but declares them neither editable nor savable, so keeping one open only buys a second parse.pdf changes which viewer a failed upload falls back to, so that stays explicit: the google docs viewer still gets the pdf, the microsoft one keeps everything else the core knows.
Verification
Rendered on an emulator, each in a fresh process (
type=from the core's own log):type=pptxtype=ppttype=doctype=xlsSheet1, table with rowsCoreTestgrows a case per format over the same samples odrcore tests with,CoreLoaderTestis new and covers the claim itself, andspotlessCheck/lintProDebug/ the unit tests are green.🤖 Generated with Claude Code