diff --git a/app/src/androidTest/assets/corrupt.odt b/app/src/androidTest/assets/corrupt.odt new file mode 100644 index 000000000000..ced8c17fde61 Binary files /dev/null and b/app/src/androidTest/assets/corrupt.odt differ diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index 67dd71ca98fa..9f861745d22c 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/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 diff --git a/app/src/main/java/app/opendocument/droid/background/LoaderService.kt b/app/src/main/java/app/opendocument/droid/background/LoaderService.kt index 297910e0de70..21357f8326d5 100644 --- a/app/src/main/java/app/opendocument/droid/background/LoaderService.kt +++ b/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) } } diff --git a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt b/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt index 09a1f2efa9bc..2d008b2db48c 100644 --- a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt +++ b/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() diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt index 831130e8799c..3eeb3f1a2f29 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt @@ -1,12 +1,17 @@ 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 @@ -14,9 +19,11 @@ 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(R.id.dialog_broken_file_contact).setOnClickListener { + contactSupport(activity) + + dialog.dismiss() + } + view.findViewById(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(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, diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index f32a826b2759..06fdd97884cd 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/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) diff --git a/app/src/main/res/layout/dialog_broken_file.xml b/app/src/main/res/layout/dialog_broken_file.xml new file mode 100644 index 000000000000..7f339af33cee --- /dev/null +++ b/app/src/main/res/layout/dialog_broken_file.xml @@ -0,0 +1,54 @@ + + + + + + + + +