Trust the core when it says a file cannot be opened

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

@@ -17,6 +17,7 @@ import androidx.test.espresso.IdlingResource
 import androidx.test.espresso.action.ViewActions.clearText
 import androidx.test.espresso.action.ViewActions.click
 import androidx.test.espresso.action.ViewActions.typeText
+import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
 import androidx.test.espresso.assertion.ViewAssertions.matches
 import androidx.test.espresso.intent.Intents
 import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction
@@ -188,6 +189,23 @@ class MainActivityTests {
         clickEditWithOverflowFallback()
     }
 
+    @Test
+    fun testCorruptODTIsNotOfferedForUpload() {
+        val activity = mainActivityActivityTestRule.activity
+
+        // a zip that still names itself odf, with a content.xml cut in half. the core claims
+        // the format and fails on the file, and that answer is taken as final: uploading it
+        // would only run the same core on a server and fail a second time
+        //
+        // not loadDocument(), which waits for a fragment this path takes back down again
+        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()))
+    }
+
     @Test
     fun testODTEditMode() {
         val activity = mainActivityActivityTestRule.activity
@@ -520,7 +538,13 @@ class MainActivityTests {
             val testAssetManager = instrumentation.context.assets
 
             for (filename in
-                arrayOf("test.odt", "dummy.pdf", "password-test.odt", "style-various-1.docx")) {
+                arrayOf(
+                    "test.odt",
+                    "dummy.pdf",
+                    "password-test.odt",
+                    "style-various-1.docx",
+                    "corrupt.odt",
+                )) {
                 val targetFile = File(testDocumentsDir, filename)
                 copy(testAssetManager.open(filename), targetFile)
                 testFiles[filename] = targetFile

app/src/main/java/app/opendocument/droid/background/LoaderService.kt

@@ -171,6 +171,11 @@ class LoaderService : Service(), FileLoader.FileLoaderListener {
 
             if (rawLoader.isSupported(options)) {
                 loadWithType(FileLoader.LoaderType.RAW, options)
+            } else if (coreLoader.isSupported(options)) {
+                // the core names this format and still said no, so the file is what is wrong.
+                // an upload would only run the same engine again - onUnsupported is below,
+                // for the formats the core never claimed
+                withListener { it.onError(result, error) }
             } else {
                 withListener { it.onUnsupported(result) }
             }

app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt

@@ -40,6 +40,11 @@ object SnackbarHelper {
                     message,
                     duration,
                 )
+
+            // material stops at two lines, which toast_error_save_failed already fills in
+            // english and overflows once translated
+            snackbar.setTextMaxLines(3)
+
             if (callback != null) {
                 snackbar.setAction(buttonText) {
                     callback.run()

app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt

@@ -1,22 +1,29 @@
 package app.opendocument.droid.ui.activity
 
 import android.app.Activity
+import android.content.ActivityNotFoundException
 import android.content.Intent
 import android.net.Uri
 import android.os.Bundle
 import android.os.Handler
 import android.os.Looper
 import android.text.InputType
+import android.text.SpannableString
+import android.text.Spanned
+import android.text.method.LinkMovementMethod
+import android.text.style.ClickableSpan
 import android.view.LayoutInflater
 import android.view.Menu
 import android.view.MenuInflater
 import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
 import android.widget.EditText
+import android.widget.TextView
 import android.widget.Toast
 import androidx.annotation.VisibleForTesting
 import androidx.appcompat.app.AlertDialog
+import androidx.core.net.toUri
 import androidx.core.view.MenuProvider
 import androidx.fragment.app.Fragment
 import androidx.lifecycle.ViewModel
@@ -511,15 +518,29 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider
         unload()
         dismissProgress()
 
-        val errorDescription =
-            when (error) {
-                is FileNotFoundException -> R.string.toast_error_find_file
-                is OutOfMemoryError -> R.string.toast_error_out_of_memory
-                else -> R.string.toast_error_generic
-            }
+        when {
+            error is FileNotFoundException ->
+                offerReopen(activity, options, R.string.toast_error_find_file, true)
+            error is OutOfMemoryError ->
+                offerReopen(activity, options, R.string.toast_error_out_of_memory, true)
+            // an upload that did not come back says nothing about the file - the network or
+            // the server is what failed, and the file was one we never claimed to open in the
+            // first place. keep the reopen offer, which is the useful thing left to do with it
+            result.loaderType == FileLoader.LoaderType.ONLINE ->
+                offerReopen(activity, options, R.string.toast_error_upload_failed, true)
+            // MetadataLoader could not read the file, or the core names its format and still
+            // could not open it. Neither is worth an upload, so ask to hear about it instead
+            else -> {
+                // nothing is ever going to be shown for this file, so drop back to the
+                // landing screen and let the dialog come up over that
+                state.endLoadIdling()
+                (activity as MainActivity).closeDocument()
+
+                offerContact(activity)
 
-        // MetadataLoader failed, so there's no point in trying to parse or upload the file
-        offerReopen(activity, options, errorDescription, true)
+                return
+            }
+        }
 
         state.endLoadIdling()
     }
@@ -636,6 +657,71 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider
         builder.show()
     }
 
+    private fun offerContact(activity: Activity) {
+        analyticsManager.report("contact_offer")
+
+        // its own content view rather than setMessage plus the builder's buttons - see
+        // dialog_broken_file.xml. every string comes off the activity, because closeDocument()
+        // has already detached this fragment and its own getString() would throw
+        val view = activity.layoutInflater.inflate(R.layout.dialog_broken_file, null)
+
+        val dialog =
+            AlertDialog.Builder(activity)
+                .setTitle(R.string.dialog_broken_file_title)
+                .setView(view)
+                .show()
+
+        view.findViewById<View>(R.id.dialog_broken_file_contact).setOnClickListener {
+            contactSupport(activity)
+
+            dialog.dismiss()
+        }
+        view.findViewById<View>(R.id.dialog_broken_file_ok).setOnClickListener {
+            dialog.dismiss()
+        }
+
+        // the address is clickable too, through the same guarded launch as the button rather
+        // than autoLink - TextView's own mailto handler throws where there is no mail app.
+        // a translation that dropped the address just leaves the message as plain text
+        val message = view.findViewById<TextView>(R.id.dialog_broken_file_message)
+        val address = activity.getString(R.string.support_email)
+        val text = SpannableString(message.text)
+        val start = text.indexOf(address)
+        if (start >= 0) {
+            text.setSpan(
+                object : ClickableSpan() {
+                    override fun onClick(widget: View) {
+                        contactSupport(activity)
+
+                        dialog.dismiss()
+                    }
+                },
+                start,
+                start + address.length,
+                Spanned.SPAN_EXCLUSIVE_EXCLUSIVE,
+            )
+
+            message.text = text
+            message.movementMethod = LinkMovementMethod.getInstance()
+        }
+    }
+
+    private fun contactSupport(activity: Activity) {
+        val intent =
+            Intent(
+                Intent.ACTION_SENDTO,
+                "mailto:${activity.getString(R.string.support_email)}".toUri(),
+            )
+        intent.putExtra(Intent.EXTRA_SUBJECT, activity.getString(R.string.app_title))
+
+        try {
+            activity.startActivity(intent)
+        } catch (e: ActivityNotFoundException) {
+            // no mail app - the address is in the dialog either way, so say nothing more
+            crashManager.log(e)
+        }
+    }
+
     private fun offerReopen(
         activity: Activity,
         options: FileLoader.Options,

app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt

@@ -659,7 +659,9 @@ class MainActivity : AppCompatActivity(), MenuProvider {
         analyticsManager.report("fullscreen_end")
     }
 
-    private fun closeDocument() {
+    // also called by DocumentFragment when a load failed for good, so the landing screen is
+    // what the error dialog comes up over rather than an empty document view
+    fun closeDocument() {
         documentFragment?.let { fragment ->
             removeMenuProvider(fragment)
 

app/src/main/res/layout/dialog_broken_file.xml

@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- The buttons are part of this view rather than the dialog's own button bar: appcompat draws
+     that bar over an opaque background of its own, which reads as a grey band under an otherwise
+     white dialog and does not go away by painting any of the views it is made of.
+
+     defaultFocusHighlightEnabled is off on everything focusable here: the framework paints a
+     translucent grey over whichever view holds focus while the window is out of touch mode,
+     and on a dialog opened without a tap that is what the grey band actually was. -->
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content"
+    android:background="#ffffff"
+    android:orientation="vertical"
+    android:paddingStart="24dp"
+    android:paddingEnd="16dp"
+    android:paddingTop="4dp"
+    android:paddingBottom="8dp">
+
+    <TextView
+        android:id="@+id/dialog_broken_file_message"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:defaultFocusHighlightEnabled="false"
+        android:paddingEnd="8dp"
+        android:text="@string/dialog_broken_file"
+        android:textColorLink="#6b6b6b"
+        android:textSize="16sp" />
+
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="12dp"
+        android:gravity="end"
+        android:orientation="horizontal">
+
+        <Button
+            android:id="@+id/dialog_broken_file_contact"
+            style="?android:attr/borderlessButtonStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:defaultFocusHighlightEnabled="false"
+            android:text="@string/action_contact"
+            android:textColor="#6b6b6b" />
+
+        <Button
+            android:id="@+id/dialog_broken_file_ok"
+            style="?android:attr/borderlessButtonStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:defaultFocusHighlightEnabled="false"
+            android:text="@android:string/ok"
+            android:textColor="#6b6b6b" />
+    </LinearLayout>
+</LinearLayout>

app/src/main/res/values/strings.xml

@@ -2,13 +2,15 @@
 <resources>
     <string name="app_title">OpenDocument Reader</string>
 
-    <string name="toast_error_generic">Something bad happened. Couldn\'t open file.</string>
+    <string name="dialog_broken_file_title">Couldn\'t open this file</string>
+    <string name="dialog_broken_file">The file may be damaged, or something went wrong on our side. If you think the file is fine, tell us at support@opendocument.app</string>
     <string name="toast_error_find_file">Couldn\'t find file. Maybe it doesn\'t exist any longer?</string>
     <string name="toast_error_illegal_file">This doesn\'t seem to be a supported file format.</string>
     <string name="toast_error_illegal_file_reopen">Unsupported file format. Try opening it in another app.</string>
     <string name="toast_hint_unsupported_file">Not happy with how the file is displayed? Open it in another app instead.</string>
     <string name="toast_error_save_nofile">No place to save the file to chosen.</string>
     <string name="toast_error_save_failed">File could not be saved. Please contact support@opendocument.app</string>
+    <string name="toast_error_upload_failed">Upload failed. Try opening the file in another app.</string>
     <string name="toast_error_out_of_memory">Device out of memory! Too many pictures, or file too big.</string>
     <string name="toast_error_password_protected">This document is password-protected</string>
     <string name="dialog_recent_title">Recent documents</string>
@@ -39,6 +41,7 @@
     <string name="action_tts_previous">Previous</string>
     <string name="action_edit_save">Save</string>
     <string name="action_upload">Upload</string>
+    <string name="action_contact">Contact</string>
     <string name="action_edit_banner">Edit your document below and press Save</string>
     <string name="landing_welcome_title">Welcome to \nOpenDocument Reader</string>
     <string name="landing_intro_view">OpenDocument Reader allows you to view documents that are stored in OpenDocument format (.odt, .ods, .odp and .odg) wherever you are.</string>
@@ -56,6 +59,9 @@
     <string name="crouton_remove_ads">Need more space to read? Remove ads for free via the menu.</string>
     <string name="reopen_chooser_title">Open using another app installed on your device:</string>
 
+    <!-- spelled out in the messages above too, where translators need it in context -->
+    <string name="support_email" translatable="false">support@opendocument.app</string>
+
     <!--  iOS -->
     <string name="ok">OK</string>
     <string name="unsaved_changes">You have unsaved changes</string>