Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,52 @@ 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, 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.

That walk has a blind spot, and a second test covers it. The table matches by *prefix*, so
it also claims the ooxml templates, slideshows and macro-enabled variants - spellings
odrcore never produces, because `mimetypeByFileType` names one canonical mime per format. An
intent-filter, on the other hand, matches a mime type exactly, so every one of those has to
be spelled out in the manifest. `theOoxmlVariantsReachUsToo` lists them and asserts both
sides say yes.

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 item 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`.

### Language

Kotlin; support is built into AGP 9, no kotlin plugin is applied. The only java left is
Expand Down
Binary file added app/src/androidTest/assets/11KB.doc
Binary file not shown.
Binary file not shown.
Binary file added app/src/androidTest/assets/style-various-1.ppt
Binary file not shown.
Binary file added app/src/androidTest/assets/style-various-1.pptx
Binary file not shown.
110 changes: 109 additions & 1 deletion app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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
Expand All @@ -66,6 +86,10 @@ class CoreTest {
passwordTestFile?.delete()
spreadsheetTestFile?.delete()
docxTestFile?.delete()
pptxTestFile?.delete()
docTestFile?.delete()
pptTestFile?.delete()
xlsTestFile?.delete()
}

@Test
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading