Drop the raw and online loaders

CHANGELOG.md

@@ -24,6 +24,10 @@ Play Console when the release is promoted.
 - Opening an archive lists its entries as files instead of one page of gibberish.
 - Embedded fonts in a PDF no longer come out as boxes or as the wrong letters.
 - svg, ico, jxl, jp2, psd, wmf and emf are recognised and shown as images.
+- An .xml file opens properly laid out instead of as one long line.
+- Smaller fixes to plain text and to the margin documents open with.
+- A file the app cannot open is no longer offered for upload to our conversion
+  service, so no document leaves your device. This affected .rtf and WordPerfect.
 
 ## 4.13.0
 

CLAUDE.md

@@ -16,9 +16,9 @@ Guidance for Claude Code (claude.ai/code) working in this repository.
 
 A `FileLoader` loads on `LoaderService`'s background thread and reports through
 `FileLoaderListener`; `LoaderServiceQueue` holds requests until the service is bound.
-`MetadataLoader` caches and identifies the file, `RawLoader` takes svg and xml,
-`CoreLoader` renders the rest odrcore handles and publishes the html on a local server, and
-`OnlineLoader` uploads to a web viewer what neither can open.
+`MetadataLoader` caches and identifies the file and `CoreLoader` renders it, publishing the
+html on a local server. Those two are the whole chain - there is no fallback after the core,
+and what it cannot open is reported as an unsupported format.
 
 `MainActivity` owns the service binding and the action modes (find, tts, edit), and swaps
 between `LandingFragment` (recent documents and settings) and `DocumentFragment`, which
@@ -143,27 +143,22 @@ XML cannot read any of that, so the `STRICT_CATCH` alias' three intent-filters a
 *generated* from the same table - a filter matches a mime type exactly, so all 49 spellings
 and 41 extensions are written out. `SupportedFormatsTest` asserts that
 `SupportedDocumentTypes` and the package manager agree, and that every claimed mime type
-reaches a loader, so a format added upstream and forgotten fails CI.
+reaches `CoreLoader`, so a format added upstream and forgotten fails CI.
 
-The tables live in `libodr_jni`, which is why `CoreLoaderTest`,
-`SupportedDocumentTypesTest`, `RawLoaderTest` and `OnlineLoaderTest` are instrumented though
-none opens a file. After caching it is `Odr.mimetype` that decides, canonicalized through
-`canonicalMimeType` so the loaders see one spelling per format.
+The tables live in `libodr_jni`, which is why `CoreLoaderTest` and
+`SupportedDocumentTypesTest` are instrumented though neither opens a file. After caching it
+is `Odr.mimetype` that decides, canonicalized through `canonicalMimeType` so the loaders see
+one spelling per format.
 
 Reading the core's table directly, as `isDocument` does, must not `lowercase()` first: it
 matches exactly and spells some types with capitals (`macroEnabled`). Our own sets are the
 other way round - `mimeTypesOf` lowercases what it stores.
 
-### `RawLoader` is asked before `CoreLoader`, not after it
+### There is nothing after `CoreLoader`
 
-`LoaderService.onSuccess` asks `rawLoader.isSupported` *first*, because the core would
-render an svg itself. `isRenderedByRaw` is the whole list: svg, which the WebView draws
-just as well from the file, and xml, which the core names without a decoder. A `RawLoader`
-failure falls through to the core; what the core cannot open goes to the upload offer
-rather than to the WebView on spec.
-
-Routing by name is guarded - see `nameSays`. The core identifies by content, so a
-`drawing.svg` holding an odt is an odt.
+The only answer to a file the core cannot open is to say so - `onUnsupported`, the reopen bar
+and the contact dialog. Do not add a route around it: a format the app should open is a format
+odrcore should learn, and rtf and WordPerfect are on that list. No document leaves the device.
 
 ### `text/plain` from the core is a guess unless a charset came with it
 
@@ -173,8 +168,8 @@ rendered, on the server thread, long after `CoreLoader` reported success.
 
 So `MetadataLoader` drops a `text/plain` whose file has no charset (`hasKnownCharset`) and
 lets the fallbacks below it decide, and `CoreLoader.host()` refuses the same file up front.
-Both are needed: the first keeps `isRenderedByCore` and `OnlineLoader`'s `"text/"` whitelist
-off a `.bin`, the second stops a success bar appearing over a page that cannot draw.
+Both are needed: the first keeps `isRenderedByCore` off a `.bin`, the second stops a success
+bar appearing over a page that cannot draw.
 `LandingTests.aDocumentThatFailsToOpenComesBackToTheList` holds this.
 
 ### Editability comes from the core, never from a mime type
@@ -200,13 +195,14 @@ recent list rather than releasing on close. Do not add a release next to
 `documentFragment.loadUri()`: that call only queues the load, so the stream is opened long
 after it returns.
 
-### Kotlin, and the three `@Jvm` annotations left
+### Kotlin, and the two `@Jvm` annotations left
 
 The only java is `com/commonsware/android/print`, vendored so it can be diffed against
 upstream. It calls nothing of ours, so no java-to-kotlin call exists and `@JvmStatic`,
 `@JvmField`, `@JvmOverloads` and `@Throws` are not needed for interop.
 
 What remains is for runtimes that reflect over the bytecode: `@JvmField` on `FileLoader`'s
-`CREATOR`s (parcelable needs a static field), `@JvmOverloads` on `ProgressDialogFragment`'s
-constructor (the framework re-creates it with no arguments), and `@JvmStatic` on
-`@BeforeClass` / `@AfterClass` in the instrumented tests.
+`CREATOR`s (parcelable needs a static field) and `@JvmStatic` on `@BeforeClass` /
+`@AfterClass` in the instrumented tests. `ProgressDialogFragment` needed `@JvmOverloads` too
+while it took an argument - a fragment the framework re-creates has to have a no-arg
+constructor.

app/src/androidTest/java/app/opendocument/droid/background/CoreLoaderTest.kt

@@ -99,27 +99,25 @@ class CoreLoaderTest {
         // rendered when handed one, never claimed in the share sheet
         assertTrue(isSupported("audio/mpeg"))
         assertTrue(isSupported("video/mp4"))
-        // the image types 6.3 added, svg among them - [RawLoader] takes that one anyway
+        // the image types 6.3 added
         assertTrue(isSupported("image/svg+xml"))
         assertTrue(isSupported("image/jxl"))
         assertTrue(isSupported("image/vnd.adobe.photoshop"))
     }
 
-    /**
-     * 6.4 opens a csv as a spreadsheet, so the table is the core's to draw now, not RawLoader's.
-     */
+    /** 6.4 opens a csv as a spreadsheet, so the table is the core's to draw. */
     @Test
     fun csvIsSupported() {
         assertTrue(isSupported("text/csv"))
         assertTrue(isSupported("application/csv"))
         assertTrue(isSupported("text/comma-separated-values"))
     }
 
-    /** Named by the core since 6.3, but with no decoder behind the name - still [RawLoader]'s. */
+    /** 6.5 puts a decoder behind the name the core gave xml in 6.3. */
     @Test
-    fun whatTheCoreNamesButCannotTranslateIsNotClaimed() {
-        assertFalse(isSupported("application/xml"))
-        assertFalse(isSupported("text/xml"))
+    fun xmlIsSupported() {
+        assertTrue(isSupported("application/xml"))
+        assertTrue(isSupported("text/xml"))
     }
 
     @Test

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

@@ -380,9 +380,9 @@ class LandingTests {
      *
      * The extension is part of the fixture. `Odr.mimetype` cannot identify these bytes, so
      * `MetadataLoader` falls back to what the provider makes of the filename - and that type is
-     * what decides which failure the user gets. `.bin` gives `application/octet-stream`, which no
-     * loader claims, so the app runs out of things to try. Name it `.odt` and the upload offer
-     * takes over instead, because `OnlineLoader` whitelists the type the name implies.
+     * what decides which failure the user gets. `.bin` gives `application/octet-stream`, which the
+     * core does not claim, so the file is reported as an unsupported format. Name it `.odt` and the
+     * core claims the format and fails on the bytes, which is the broken-file dialog instead.
      */
     private fun seedBrokenDocument() {
         val broken = File(requireTestFile().parentFile, BROKEN_DOCUMENT)

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

@@ -163,15 +163,14 @@ class MainActivityTests {
     }
 
     @Test
-    fun testCorruptODTIsNotOfferedForUpload() {
+    fun testCorruptODTOffersContact() {
         val activity = mainActivityActivityTestRule.activity
 
-        // the core claims the format and fails on the file, which is final - an upload runs the
-        // same core. not loadDocument(), which waits for a fragment this path takes back down
+        // the core claims the format and fails on the file, which is final. not loadDocument(),
+        // which waits for a fragment this path takes back down
         val testFileUri = uriOf(requireTestFile("corrupt.odt"))
         InstrumentationRegistry.getInstrumentation().runOnMainSync { activity.loadUri(testFileUri) }
 
-        onView(withText(R.string.action_upload)).check(doesNotExist())
         onView(withText(R.string.dialog_broken_file)).check(matches(isDisplayed()))
         onView(withText(R.string.action_contact)).check(matches(isDisplayed()))
     }