From a16f5547990d81652006e8b7a5882aaaca439aa6 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Fri, 19 Feb 2021 16:12:57 -0500 Subject: [PATCH] For issue #10428 Improve download dialog error message. --- .../fenix/browser/BaseBrowserFragment.kt | 35 +++++++++--------- .../fenix/downloads/DynamicDownloadDialog.kt | 17 +++++++-- .../downloads/DynamicDownloadDialogTest.kt | 36 +++++++++++++++++++ 3 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/downloads/DynamicDownloadDialogTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index ecc49681e8..5a925e49d9 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -474,13 +474,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit didFail = downloadJobStatus == DownloadState.Status.FAILED, tryAgain = downloadFeature::tryAgain, onCannotOpenFile = { - FenixSnackbar.make( - view = view.browserLayout, - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = true - ) - .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file)) - .show() + showCannotOpenFileError(view.browserLayout, context, it) }, view = view.viewDynamicDownloadDialog, toolbarHeight = toolbarHeight, @@ -783,16 +777,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit } } - val onCannotOpenFile = { - FenixSnackbar.make( - view = view.browserLayout, - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = true - ) - .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file)) - .show() - } - val onDismiss: () -> Unit = { sharedViewModel.downloadDialogState.remove(sessionId) } @@ -802,7 +786,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit metrics = requireComponents.analytics.metrics, didFail = savedDownloadState.second, tryAgain = onTryAgain, - onCannotOpenFile = onCannotOpenFile, + onCannotOpenFile = { + showCannotOpenFileError(view.browserLayout, context, it) + }, view = view.viewDynamicDownloadDialog, toolbarHeight = toolbarHeight, onDismiss = onDismiss @@ -1280,6 +1266,19 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit ) } + private fun showCannotOpenFileError( + view: View, + context: Context, + downloadState: DownloadState + ) { + FenixSnackbar.make( + view = view, + duration = Snackbar.LENGTH_SHORT, + isDisplayedWithBrowserToolbar = true + ).setText(DynamicDownloadDialog.getCannotOpenFileErrorMessage(context, downloadState)) + .show() + } + companion object { private const val KEY_CUSTOM_TAB_SESSION_ID = "custom_tab_session_id" private const val REQUEST_CODE_DOWNLOAD_PERMISSIONS = 1 diff --git a/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt index db524f182c..8f2b4fa395 100644 --- a/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt +++ b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt @@ -4,8 +4,10 @@ package org.mozilla.fenix.downloads +import android.content.Context import android.view.View import android.view.ViewGroup +import android.webkit.MimeTypeMap import androidx.coordinatorlayout.widget.CoordinatorLayout import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.download_dialog_layout.view.* @@ -30,7 +32,7 @@ class DynamicDownloadDialog( private val metrics: MetricController, private val didFail: Boolean, private val tryAgain: (String) -> Unit, - private val onCannotOpenFile: () -> Unit, + private val onCannotOpenFile: (DownloadState) -> Unit, private val view: View, private val toolbarHeight: Int, private val onDismiss: () -> Unit @@ -110,7 +112,7 @@ class DynamicDownloadDialog( ) if (!fileWasOpened) { - onCannotOpenFile() + onCannotOpenFile(downloadState) } context.metrics.track(Event.InAppNotificationDownloadOpen) @@ -138,4 +140,15 @@ class DynamicDownloadDialog( view.visibility = View.GONE onDismiss() } + + companion object { + fun getCannotOpenFileErrorMessage(context: Context, download: DownloadState): String { + val fileExt = MimeTypeMap.getFileExtensionFromUrl( + download.filePath + ) + return context.getString( + R.string.mozac_feature_downloads_open_not_supported1, fileExt + ) + } + } } diff --git a/app/src/test/java/org/mozilla/fenix/downloads/DynamicDownloadDialogTest.kt b/app/src/test/java/org/mozilla/fenix/downloads/DynamicDownloadDialogTest.kt new file mode 100644 index 0000000000..4ac91f8379 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/downloads/DynamicDownloadDialogTest.kt @@ -0,0 +1,36 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.downloads + +import android.webkit.MimeTypeMap +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.downloads.DynamicDownloadDialog.Companion.getCannotOpenFileErrorMessage +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.robolectric.Shadows.shadowOf + +@RunWith(FenixRobolectricTestRunner::class) +class DynamicDownloadDialogTest { + + @Test + fun `WHEN calling getCannotOpenFileErrorMessage THEN should return the error message for the download file type`() { + val download = DownloadState(url = "", fileName = "image.gif") + + shadowOf(MimeTypeMap.getSingleton()).apply { + addExtensionMimeTypMapping(".gif", "image/gif") + } + + val expected = testContext.getString( + R.string.mozac_feature_downloads_open_not_supported1, "gif" + ) + + val result = getCannotOpenFileErrorMessage(testContext, download) + assertEquals(expected, result) + } +}