Open every format the core renders, not just half of them

CLAUDE.md

@@ -145,17 +145,43 @@ the `.pro` suffix) differ on purpose - do not "fix" the mismatch:
   `AndroidFileCache`, the SharedPreferences name in `MainActivity`) follows
   `applicationId` and must stay that way, or existing users lose their saved prefs.
 
-### Supported file types are declared twice
-
-`SupportedDocumentTypes` (mime prefixes plus an extension fallback) and the `STRICT_CATCH`
-`activity-alias` in `AndroidManifest.xml` describe the same set from two directions - the manifest
-is what the system offers the app for, `SupportedDocumentTypes` is what the folder browser on the
-landing screen offers itself for. **Nothing keeps them in step, so change both.**
-
-The duplication is deliberate: `MetadataLoader` decides what is really supported by running
-libmagic over the file *after* copying it into the cache, and `CoreLoader.isSupported()` /
-`RawLoader.isSupported()` both take a filled-in `FileLoader.Options`. A folder listing only has a
-name and whatever mime type the provider volunteered, so it cannot ask them and has to guess.
+### Supported file types are declared twice, and a test keeps them in step
+
+`SupportedDocumentTypes` is the single kotlin table: mime prefixes for what the core renders, mime
+prefixes for what `RawLoader` shows, plus an extension fallback for the `application/octet-stream`
+a provider volunteers when it knows nothing better. `CoreLoader.isSupported()` defers to it - it
+used to keep a second copy of the same prefixes.
+
+The `STRICT_CATCH` `activity-alias` in `AndroidManifest.xml` is the second declaration and cannot be
+collapsed into the first, because XML cannot read a kotlin list. It is covered instead:
+`SupportedFormatsTest` (instrumented) walks odrcore's own `FileType` values, asks
+`Odr.mimetypeByFileType` for each, and asserts that `SupportedDocumentTypes` and the package
+manager give the same answer - so a format added on one side and forgotten on the other fails CI
+rather than shipping.
+
+The set follows odrcore: whatever it keeps reference html output for in `test/data/reference-output`
+is what the app claims - odf, ooxml, the legacy binary doc/ppt/xls and pdf. Text, csv and images are
+the core's too but belong to `RawLoader`, which only gets its turn when `CoreLoader.isSupported()`
+says no.
+
+Why the app has a table at all when odrcore has `Odr.fileTypeByMimetype` /
+`Odr.fileTypeByFileExtension`: both are native calls into `libodr_jni`, and both know exactly one
+canonical mime type per format - no templates, no macro-enabled variants, no
+`application/x-vnd.oasis...`, which are the spellings content providers actually hand out. The
+guessing stage needs to be broader than the core's own lookup. What the core *does* decide is
+everything after the file is in the cache: `MetadataLoader` runs libmagic over the copy, and
+`CoreLoader.isDocumentEditable` asks the opened document.
+
+### Editability comes from the core, never from a mime type
+
+`Document.isEditable()`/`isSavable()` is what decides whether `DocumentFragment` puts the Edit
+button up, carried across on `FileLoader.Result.isEditable`. `CoreLoader.host()` only holds a
+document open when the core says yes, so "we have a document" *is* the answer.
+
+Do not reintroduce a list of editable formats in the UI. The core already says no to the legacy
+binary doc/ppt/xls, to ooxml spreadsheets and presentations, and to every spreadsheet including odf
+(its own TODO, the same gap as issue #442) - all of which the app used to spell out by mime prefix
+and got wrong the moment a new format started going through `CoreLoader`.
 
 ### Storage access
 

app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt

@@ -31,6 +31,10 @@ class CoreTest {
     private var passwordTestFile: File? = null
     private var spreadsheetTestFile: File? = null
     private var docxTestFile: File? = null
+    private var pptxTestFile: File? = null
+    private var docTestFile: File? = null
+    private var pptTestFile: File? = null
+    private var xlsTestFile: File? = null
 
     private val coreLoader: CoreLoader
         get() = checkNotNull(sharedLoader) { "the core loader was not started" }
@@ -58,6 +62,22 @@ class CoreTest {
                 assetManager.open("style-various-1.docx"),
                 File(appCtx.cacheDir, "style-various-1.docx"),
             )
+        pptxTestFile =
+            extract(
+                assetManager.open("style-various-1.pptx"),
+                File(appCtx.cacheDir, "style-various-1.pptx"),
+            )
+        docTestFile = extract(assetManager.open("11KB.doc"), File(appCtx.cacheDir, "11KB.doc"))
+        pptTestFile =
+            extract(
+                assetManager.open("style-various-1.ppt"),
+                File(appCtx.cacheDir, "style-various-1.ppt"),
+            )
+        xlsTestFile =
+            extract(
+                assetManager.open("file_example_XLS_10.xls"),
+                File(appCtx.cacheDir, "file_example_XLS_10.xls"),
+            )
     }
 
     @After
@@ -66,6 +86,10 @@ class CoreTest {
         passwordTestFile?.delete()
         spreadsheetTestFile?.delete()
         docxTestFile?.delete()
+        pptxTestFile?.delete()
+        docTestFile?.delete()
+        pptTestFile?.delete()
+        xlsTestFile?.delete()
     }
 
     @Test
@@ -109,6 +133,90 @@ class CoreTest {
         Assert.assertTrue("the edited document should have been saved", result.isFile)
     }
 
+    /**
+     * The formats that used to be sent off for conversion instead: presentations, and the legacy
+     * binary microsoft ones. The core renders all four, so the app hands them to it.
+     */
+    @Test
+    fun testPptx() {
+        val views =
+            coreLoader.host(
+                prefix = "pptx-test",
+                inputPath = requireFile(pptxTestFile).absolutePath,
+                cachePath = File(cacheDir(), "pptx_cache").path,
+            )
+        Assert.assertFalse("hosting the PPTX file should produce a view", views.isEmpty())
+    }
+
+    @Test
+    fun testDoc() {
+        val views =
+            coreLoader.host(
+                prefix = "doc-test",
+                inputPath = requireFile(docTestFile).absolutePath,
+                cachePath = File(cacheDir(), "doc_cache").path,
+            )
+        Assert.assertFalse("hosting the DOC file should produce a view", views.isEmpty())
+    }
+
+    @Test
+    fun testPpt() {
+        val views =
+            coreLoader.host(
+                prefix = "ppt-test",
+                inputPath = requireFile(pptTestFile).absolutePath,
+                cachePath = File(cacheDir(), "ppt_cache").path,
+            )
+        Assert.assertFalse("hosting the PPT file should produce a view", views.isEmpty())
+    }
+
+    @Test
+    fun testXls() {
+        val views =
+            coreLoader.host(
+                prefix = "xls-test",
+                inputPath = requireFile(xlsTestFile).absolutePath,
+                cachePath = File(cacheDir(), "xls_cache").path,
+            )
+        Assert.assertFalse("hosting the XLS file should produce a view", views.isEmpty())
+    }
+
+    /**
+     * Which of the formats the core renders it can also write back again - the answer
+     * `DocumentFragment` puts the Edit button up by. Offering it for a document the core will not
+     * save only gets as far as a failed save, which is what happened to the legacy binary formats
+     * when they started going through the core.
+     */
+    @Test
+    fun testEditableFormats() {
+        assertEditable("odt-editable", requireFile(testFile), true)
+        assertEditable("docx-editable", requireFile(docxTestFile), true)
+
+        // the core declares these read only: the three legacy binary formats, ooxml presentations
+        // and every spreadsheet - the last being issue #442, which the core has its own TODO for
+        assertEditable("doc-editable", requireFile(docTestFile), false)
+        assertEditable("ppt-editable", requireFile(pptTestFile), false)
+        assertEditable("xls-editable", requireFile(xlsTestFile), false)
+        assertEditable("pptx-editable", requireFile(pptxTestFile), false)
+        assertEditable("ods-editable", requireFile(spreadsheetTestFile), false)
+    }
+
+    private fun assertEditable(prefix: String, file: File, expected: Boolean) {
+        coreLoader.host(
+            prefix = prefix,
+            inputPath = file.absolutePath,
+            cachePath = File(cacheDir(), prefix).path,
+            editable = true,
+            keepDocument = true,
+        )
+
+        Assert.assertEquals(
+            "the core should report ${file.name} as ${if (expected) "editable" else "read only"}",
+            expected,
+            coreLoader.isDocumentEditable,
+        )
+    }
+
     @Test
     fun testPasswordProtectedDocumentWithoutPassword() {
         Assert.assertThrows(OdrException.FileEncrypted::class.java) {
@@ -176,7 +284,7 @@ class CoreTest {
             // set of statics. nothing here goes through loadAsync, so both handlers can be the main
             // looper and the listener is never called back
             val handler = Handler(Looper.getMainLooper())
-            val loader = CoreLoader(appCtx, true)
+            val loader = CoreLoader(appCtx)
             sharedLoader = loader
             loader.initialize(
                 object : FileLoader.FileLoaderListener {

app/src/androidTest/java/app/opendocument/droid/test/SupportedFormatsTest.kt

@@ -0,0 +1,163 @@
+package app.opendocument.droid.test
+
+import android.content.Intent
+import android.content.pm.PackageManager
+import android.net.Uri
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.SmallTest
+import androidx.test.platform.app.InstrumentationRegistry
+import app.opendocument.core.FileType
+import app.opendocument.core.Odr
+import app.opendocument.droid.background.CatchAllSetting
+import app.opendocument.droid.background.CoreLoader
+import app.opendocument.droid.background.SupportedDocumentTypes
+import org.junit.Assert
+import org.junit.BeforeClass
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Keeps the two remaining declarations of what the app opens in step with each other, so that
+ * changing one and forgetting the other is a failing test rather than a bug report.
+ *
+ * [SupportedDocumentTypes] is one of them. The other is the STRICT_CATCH `activity-alias` in
+ * AndroidManifest.xml, which is XML and cannot read a kotlin list - but it can be *asked*: an
+ * intent-filter is only worth anything if the package manager resolves a real intent through it,
+ * which is what [resolvesToUs] does here. There used to be a third copy in `CoreLoader`; that one
+ * is gone, it defers to [SupportedDocumentTypes] now.
+ *
+ * The mime side needs no list of its own at all: [FileType] is the core's complete set of formats
+ * and [Odr.mimetypeByFileType] names each one, so the test walks what odrcore knows and demands
+ * that the app and the manifest give the same answer for every entry.
+ */
+@SmallTest
+@RunWith(AndroidJUnit4::class)
+class SupportedFormatsTest {
+
+    /**
+     * For every format odrcore can name a mime type for, what the app offers itself for and what
+     * the manifest offers it for have to be the same answer.
+     *
+     * This is the assertion that catches a format being added to the loader without being added to
+     * the manifest, which is the failure the user sees as "the app is not in the share sheet".
+     */
+    @Test
+    fun theManifestClaimsExactlyWhatTheAppClaims() {
+        var checked = 0
+
+        for (fileType in FileType.values()) {
+            val mimeType = mimeTypeOrNull(fileType) ?: continue
+            checked++
+
+            val claimedByApp = SupportedDocumentTypes.isSupported(mimeType, null)
+            val claimedByManifest = resolvesToUs(mimeType, "document")
+
+            Assert.assertEquals(
+                "$fileType ($mimeType): the app says $claimedByApp, the manifest says" +
+                    " $claimedByManifest - AndroidManifest.xml and SupportedDocumentTypes have" +
+                    " drifted apart",
+                claimedByApp,
+                claimedByManifest,
+            )
+        }
+
+        // a guard on the guard: if the core ever stops naming mime types the loop above would
+        // silently assert nothing at all
+        Assert.assertTrue("no file type was checked", checked > 10)
+    }
+
+    /** Every extension the folder browser offers for has to be one odrcore actually recognises. */
+    @Test
+    fun theExtensionFallbackNamesFormatsTheCoreKnows() {
+        for (extension in SupportedDocumentTypes.EXTENSIONS) {
+            Assert.assertNotEquals(
+                "odrcore does not know the extension .$extension",
+                FileType.UNKNOWN,
+                Odr.fileTypeByFileExtension(extension),
+            )
+        }
+    }
+
+    /**
+     * The case the extension fallback exists for: a provider that volunteers nothing better than
+     * `application/octet-stream`. The app goes by the name, and the manifest has to as well - it
+     * claims `application/octet-stream` outright, so this is really a check that the claim stays.
+     */
+    @Test
+    fun anOctetStreamWithAKnownExtensionReachesUs() {
+        for (extension in SupportedDocumentTypes.EXTENSIONS) {
+            Assert.assertTrue(
+                "the app does not offer for document.$extension",
+                SupportedDocumentTypes.isSupported(
+                    "application/octet-stream",
+                    "document.$extension",
+                ),
+            )
+            Assert.assertTrue(
+                "the manifest does not offer for document.$extension",
+                resolvesToUs("application/octet-stream", "document.$extension"),
+            )
+        }
+    }
+
+    /** What the app deliberately stays out of - the reason the catch-all filter defaults to off. */
+    @Test
+    fun unrelatedTypesReachNeither() {
+        for (mimeType in listOf("text/vcard", "text/calendar", "audio/mpeg", "video/mp4")) {
+            Assert.assertFalse(
+                "the app should not offer for $mimeType",
+                SupportedDocumentTypes.isSupported(mimeType, null),
+            )
+            Assert.assertFalse(
+                "the manifest should not offer for $mimeType",
+                resolvesToUs(mimeType, "document"),
+            )
+        }
+    }
+
+    /**
+     * Whether an `ACTION_VIEW` for this mime type and filename lands on our own activity - the only
+     * thing an intent-filter is actually good for, and the one question a hand written XML list can
+     * still be asked.
+     */
+    private fun resolvesToUs(mimeType: String, filename: String): Boolean {
+        val context = InstrumentationRegistry.getInstrumentation().targetContext
+
+        val intent =
+            Intent(Intent.ACTION_VIEW).apply {
+                addCategory(Intent.CATEGORY_DEFAULT)
+                setDataAndType(Uri.parse("content://app.opendocument.test/$filename"), mimeType)
+            }
+
+        return context.packageManager
+            .queryIntentActivities(intent, PackageManager.MATCH_DEFAULT_ONLY)
+            .any { it.activityInfo?.packageName == context.packageName }
+    }
+
+    /** Null for the types odrcore has no mime type for - word perfect, rtf, cfb and the like. */
+    private fun mimeTypeOrNull(fileType: FileType): String? =
+        try {
+            Odr.mimetypeByFileType(fileType)
+        } catch (e: RuntimeException) {
+            null
+        }
+
+    companion object {
+
+        // @JvmStatic because junit requires @BeforeClass to be static
+        @JvmStatic
+        @BeforeClass
+        fun prepare() {
+            val context = InstrumentationRegistry.getInstrumentation().targetContext
+
+            // Odr's tables live in libodr_jni; nothing here opens a file, so the data paths that
+            // initializeCore also sets up are along for the ride rather than needed
+            CoreLoader.initializeCore(context)
+
+            // STRICT_CATCH is what this test is about, and the component states survive a test run
+            // - a previous one that flipped the switch would otherwise leave CATCH_ALL answering
+            // for everything. False is the shipped default, so nothing has to be put back.
+            CatchAllSetting.setEnabled(context, false)
+        }
+    }
+}