From 2258cfabcbdfa6e792f2754b5d0f15e0d7c13007 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 8 Mar 2024 16:13:50 +0530 Subject: [PATCH 1/5] Fixed: Some EPUB files are not downloading. * The issue was epub fileName containing the colon ":" in it that is not supported by most of fileSystem, that's why it is not creating the file in fileSystem and we are not able to download the epub file. * So to fix this we have improved our `getDecodedFileName` method which returns the fileName of the epub file, here we are removing the colon from fileName if any contains. For this change we have added the test cases as well for our `getDecodedFileName` function to properly test it. * We also refined our downloadFileFromUrl method. Previously, the generateSequence function was used to create new files with underscores and incremented numbers, anticipating multiple attempts to save the same file. However, since we now save files only once in our storage, this feature is no longer utilized. This enhancement is detailed in issue #2879. * Added epub query in our manifest to properly open epub files in external application. --- .../files/FileUtilsInstrumentationTest.kt | 8 ++++++++ core/src/main/AndroidManifest.xml | 6 ++++++ .../kiwixmobile/core/utils/files/FileUtils.kt | 18 ++++++------------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt index 2079286f5..b2a5fc3c1 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/files/FileUtilsInstrumentationTest.kt @@ -260,6 +260,14 @@ class FileUtilsInstrumentationTest { DummyUrlData( "https://kiwix.org/contributors/images/wikipedia", null + ), + DummyUrlData( + "https://kiwix.org/contributors/images/wikipedia:hello.epub", + "wikipediahello.epub" + ), + DummyUrlData( + "https://kiwix.org/contributors/Y Gododin: A Poem of the Battle: of Cattraeth.9842.epub", + "Y Gododin A Poem of the Battle of Cattraeth.9842.epub" ) ) dummyUrlArray.forEach { diff --git a/core/src/main/AndroidManifest.xml b/core/src/main/AndroidManifest.xml index 1f8379679..0c0ce3e85 100644 --- a/core/src/main/AndroidManifest.xml +++ b/core/src/main/AndroidManifest.xml @@ -39,6 +39,12 @@ + + + + + + diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt index 3acc0833b..c3e1d8ef8 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt @@ -410,12 +410,15 @@ object FileUtils { * We are placing a condition here for if the file name does not have a .bin extension, then it returns the original file name. + * Remove colon if any contains in the fileName since most of the fileSystem + will not allow to create file which contains colon in it. + see https://github.com/kiwix/kiwix-android/issues/3737 */ fun getDecodedFileName(url: String?): String? { var fileName: String? = null val decodedFileName = URLUtil.guessFileName(url, null, null) if (!decodedFileName.endsWith(".bin")) { - fileName = decodedFileName + fileName = decodedFileName.replace(":", "") } return fileName } @@ -444,17 +447,8 @@ object FileUtils { ) if (!root.isFileExist()) root.mkdir() } - if (File(root, fileName).isFileExist()) return File(root, fileName) - val fileToSave = sequence { - yield(File(root, fileName)) - yieldAll( - generateSequence(1) { it + 1 }.map { - File( - root, fileName.replace(".", "_$it.") - ) - } - ) - }.first { !it.isFileExist() } + val fileToSave = File(root, fileName) + if (fileToSave.isFileExist()) return fileToSave val source = if (url == null) Uri.parse(src) else Uri.parse(url) return try { zimReaderContainer.load("$source", emptyMap()).data.use { inputStream -> From 87947748160ee344ed72714906ba14c93c0fc957 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 8 Mar 2024 19:00:49 +0530 Subject: [PATCH 2/5] Introduced a Open/Download dialog for Epub/Pdf files. * This was requested in #3453 and this PR is related to the Epub opening issue so we are introducing this here. --- .../destination/reader/KiwixReaderFragment.kt | 2 +- .../core/main/CoreReaderFragment.kt | 11 ++- .../core/main/CoreWebViewClient.kt | 28 +----- .../kiwixmobile/core/main/WebViewCallback.kt | 1 + .../dialog/DownloadOrOpenEpubAndPdfHandler.kt | 87 +++++++++++++++++++ .../core/utils/dialog/KiwixDialog.kt | 8 ++ core/src/main/res/values/strings.xml | 2 + 7 files changed, 111 insertions(+), 28 deletions(-) create mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index 8beda678a..59d8190af 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -243,7 +243,7 @@ class KiwixReaderFragment : CoreReaderFragment() { override fun createWebView(attrs: AttributeSet?): ToolbarScrollingKiwixWebView { return ToolbarScrollingKiwixWebView( requireContext(), this, attrs!!, activityMainRoot as ViewGroup, videoView!!, - CoreWebViewClient(this, zimReaderContainer!!, sharedPreferenceUtil!!), + CoreWebViewClient(this, zimReaderContainer!!), toolbarContainer!!, bottomToolbar!!, sharedPreferenceUtil = sharedPreferenceUtil!!, parentNavigationBar = requireActivity().findViewById(R.id.bottom_nav_view) ) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 8d43a36da..9a270156b 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -156,6 +156,7 @@ import org.kiwix.kiwixmobile.core.utils.TAG_FILE_SEARCHED_NEW_TAB import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX import org.kiwix.kiwixmobile.core.utils.UpdateUtils.reformatProviderUrl import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower +import org.kiwix.kiwixmobile.core.utils.dialog.DownloadOrOpenEpubAndPdfHandler import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog import org.kiwix.kiwixmobile.core.utils.files.FileUtils.deleteCachedFiles import org.kiwix.kiwixmobile.core.utils.files.FileUtils.readFile @@ -321,6 +322,10 @@ abstract class CoreReaderFragment : @JvmField @Inject var externalLinkOpener: ExternalLinkOpener? = null + + @JvmField + @Inject + var downloadOrOpenEpubAndPdfHandler: DownloadOrOpenEpubAndPdfHandler? = null private var hideBackToTopTimer: CountDownTimer? = null private var documentSections: MutableList? = null private var isBackToTopEnabled = false @@ -1214,7 +1219,7 @@ abstract class CoreReaderFragment : return if (activityMainRoot != null) { ToolbarScrollingKiwixWebView( requireActivity(), this, attrs!!, (activityMainRoot as ViewGroup?)!!, videoView!!, - CoreWebViewClient(this, zimReaderContainer!!, sharedPreferenceUtil!!), + CoreWebViewClient(this, zimReaderContainer!!), toolbarContainer!!, bottomToolbar!!, sharedPreferenceUtil!! ) @@ -1542,6 +1547,10 @@ abstract class CoreReaderFragment : externalLinkOpener?.openExternalUrl(intent) } + override fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) { + downloadOrOpenEpubAndPdfHandler?.showDownloadOrOpenEpubAndPdfDialog(url, documentType) + } + protected fun openZimFile( file: File?, isCustomApp: Boolean = false, diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt index cb4ceaf95..b49625355 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt @@ -17,7 +17,6 @@ */ package org.kiwix.kiwixmobile.core.main -import android.content.Context import android.content.Intent import android.net.Uri import org.kiwix.kiwixmobile.core.utils.files.Log @@ -27,18 +26,14 @@ import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse import android.webkit.WebView import android.webkit.WebViewClient -import androidx.core.content.FileProvider import org.kiwix.kiwixmobile.core.CoreApp.Companion.instance import org.kiwix.kiwixmobile.core.reader.ZimFileReader import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer -import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX -import org.kiwix.kiwixmobile.core.utils.files.FileUtils.downloadFileFromUrl open class CoreWebViewClient( protected val callback: WebViewCallback, - protected val zimReaderContainer: ZimReaderContainer, - private val sharedPreferenceUtil: SharedPreferenceUtil + protected val zimReaderContainer: ZimReaderContainer ) : WebViewClient() { private var urlWithAnchor: String? = null @@ -85,26 +80,7 @@ open class CoreWebViewClient( private fun handleEpubAndPdf(url: String): Boolean { val extension = MimeTypeMap.getFileExtensionFromUrl(url) if (DOCUMENT_TYPES.containsKey(extension)) { - downloadFileFromUrl( - url, - null, - zimReaderContainer, - sharedPreferenceUtil - )?.let { - if (it.exists()) { - val context: Context = instance - val uri = FileProvider.getUriForFile( - context, - context.packageName + ".fileprovider", it - ) - val intent = Intent(Intent.ACTION_VIEW).apply { - setDataAndType(uri, DOCUMENT_TYPES[extension]) - flags = Intent.FLAG_ACTIVITY_NO_HISTORY - addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - } - callback.openExternalUrl(intent) - } - } + callback.showDownloadOrOpenEpubAndPdfDialog(url, DOCUMENT_TYPES[extension]) return true } return false diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt index d15eca03a..6b5ef57df 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt @@ -25,6 +25,7 @@ interface WebViewCallback { fun webViewUrlFinishedLoading() fun webViewFailedLoading(failingUrl: String) fun openExternalUrl(intent: Intent) + fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) fun webViewProgressChanged(progress: Int, webView: WebView) fun webViewTitleUpdated(title: String) fun webViewPageChanged(page: Int, maxPages: Int) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt new file mode 100644 index 000000000..230b4b000 --- /dev/null +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt @@ -0,0 +1,87 @@ +/* + * Kiwix Android + * Copyright (c) 2024 Kiwix + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package org.kiwix.kiwixmobile.core.utils.dialog + +import android.app.Activity +import android.content.Intent +import android.util.Log +import androidx.core.content.FileProvider +import org.kiwix.kiwixmobile.core.R +import org.kiwix.kiwixmobile.core.extensions.toast +import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer +import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil +import org.kiwix.kiwixmobile.core.utils.files.FileUtils.downloadFileFromUrl +import java.io.File +import javax.inject.Inject + +class DownloadOrOpenEpubAndPdfHandler @Inject constructor( + private val activity: Activity, + private val sharedPreferenceUtil: SharedPreferenceUtil, + private val alertDialogShower: AlertDialogShower, + private val zimReaderContainer: ZimReaderContainer +) { + + fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + { openOrDownloadFile(url, documentType, true) }, + { openOrDownloadFile(url, documentType, false) }, + { } + ) + } + + private fun openOrDownloadFile(url: String, documentType: String?, openFile: Boolean) { + downloadFileFromUrl( + url, + null, + zimReaderContainer, + sharedPreferenceUtil + )?.let { savedFile -> + if (openFile) { + openFile(savedFile, documentType) + } else { + activity.toast(activity.getString(R.string.save_media_saved, savedFile.name)).also { + Log.e("DownloadOrOpenEpubAndPdf", "File downloaded at = ${savedFile.path}") + } + } + } ?: run { + activity.toast(R.string.save_media_error) + } + } + + private fun openFile(savedFile: File, documentType: String?) { + if (savedFile.exists()) { + val uri = FileProvider.getUriForFile( + activity, + "${activity.packageName}.fileprovider", + savedFile + ) + val intent = Intent(Intent.ACTION_VIEW).apply { + setDataAndType(uri, documentType) + flags = Intent.FLAG_ACTIVITY_NO_HISTORY + addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) + } + if (intent.resolveActivity(activity.packageManager) != null) { + activity.startActivity(intent) + } else { + activity.toast(R.string.no_reader_application_installed) + } + } + } +} diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt index e0e1bbc7d..e00015192 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt @@ -108,6 +108,14 @@ sealed class KiwixDialog( cancelable = false ) + object DownloadOrOpenEpubAndPdf : KiwixDialog( + R.string.download_or_open_pdf_and_epub_content_dialog_title, + R.string.download_or_open_pdf_and_epub_content_dialog_message, + R.string.open, + R.string.download, + neutralMessage = R.string.no_thanks + ) + data class ShowHotspotDetails(override val args: List) : KiwixDialog( R.string.hotspot_turned_on, diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml index 84b9750bf..2df474630 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -373,4 +373,6 @@ ZIM file which has the reading content Selecting this language will prioritize displaying downloadable books in that language at the top. Go to previous screen + Download or Open this (Epub/Pdf) file? + Choosing Open will open this file in your external Epub/PDF reader application. From 38fb31b6e023b59a787795a2787d37ba44374a59 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 20 Mar 2024 17:32:11 +0530 Subject: [PATCH 3/5] Added unit test cases for testing this functionality with all possible scenarios e.g. "Download Failed", "Download Successfull", "When no reader application is installed in device", "File opens in external reader application", "When user clicks on NO Thanks button". --- .../DownloadOrOpenEpubAndPdfHandlerTest.kt | 201 ++++++++++++++++++ .../dialog/DownloadOrOpenEpubAndPdfHandler.kt | 7 +- 2 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt new file mode 100644 index 000000000..0a1dd8ad6 --- /dev/null +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt @@ -0,0 +1,201 @@ +/* + * Kiwix Android + * Copyright (c) 2024 Kiwix + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package org.kiwix.kiwixmobile.utils + +import android.app.Activity +import android.webkit.WebResourceResponse +import android.widget.Toast +import io.mockk.every +import io.mockk.justRun +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.verify +import org.junit.Before +import org.junit.Test +import org.kiwix.kiwixmobile.core.R +import org.kiwix.kiwixmobile.core.extensions.toast +import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer +import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil +import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower +import org.kiwix.kiwixmobile.core.utils.dialog.DownloadOrOpenEpubAndPdfHandler +import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog +import java.io.File +import java.io.InputStream + +class DownloadOrOpenEpubAndPdfHandlerTest { + private val demoUrl = "content://demoPdf.pdf" + private val demoFileName = "demoPdf.pdf" + private val sharedPreferenceUtil: SharedPreferenceUtil = mockk() + private val zimReaderContainer: ZimReaderContainer = mockk() + private val alertDialogShower: AlertDialogShower = mockk(relaxed = true) + private val savedFile: File = mockk(relaxed = true) + private val activity: Activity = mockk() + private val webResourceResponse: WebResourceResponse = mockk() + private val inputStream: InputStream = mockk() + private val downloadOrOpenEpubAndPdfHandler = DownloadOrOpenEpubAndPdfHandler( + activity, + sharedPreferenceUtil, + alertDialogShower, + zimReaderContainer + ) + + @Before + fun before() { + every { savedFile.name } returns demoFileName + every { sharedPreferenceUtil.isPlayStoreBuildWithAndroid11OrAbove() } returns true + every { inputStream.read(array()) } returns 1024 + every { webResourceResponse.data } returns inputStream + every { + zimReaderContainer.load( + demoUrl, + emptyMap() + ) + } returns webResourceResponse + every { activity.packageManager } returns mockk() + every { activity.packageName } returns "org.kiwix.kiwixmobile" + every { + activity.getString( + R.string.save_media_saved, + demoFileName + ) + } returns "Saved media as $demoFileName to Downloads/org.kiwix…/" + every { savedFile.path } returns "Emulated/0/Downloads/$demoFileName" + every { savedFile.exists() } returns true + downloadOrOpenEpubAndPdfHandler.intent = mockk() + every { downloadOrOpenEpubAndPdfHandler.intent.setDataAndType(any(), any()) } returns mockk() + every { downloadOrOpenEpubAndPdfHandler.intent.setFlags(any()) } returns mockk() + every { downloadOrOpenEpubAndPdfHandler.intent.addFlags(any()) } returns mockk() + } + + @Test + fun testOpeningFileInExternalReaderApplication() { + every { + downloadOrOpenEpubAndPdfHandler.intent.resolveActivity(activity.packageManager) + } returns mockk() + every { activity.startActivity(downloadOrOpenEpubAndPdfHandler.intent) } returns mockk() + val lambdaSlot = slot<() -> Unit>() + downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + verify { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + capture(lambdaSlot), + any(), + any() + ) + } + lambdaSlot.captured.invoke() + verify { + activity.startActivity(downloadOrOpenEpubAndPdfHandler.intent) + } + } + + @Test + fun testOpeningFileWhenNoReaderApplicationInstalled() { + every { + downloadOrOpenEpubAndPdfHandler.intent.resolveActivity(activity.packageManager) + } returns null + mockkStatic(Toast::class) + justRun { + Toast.makeText(activity, R.string.no_reader_application_installed, Toast.LENGTH_LONG).show() + } + val lambdaSlot = slot<() -> Unit>() + downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + verify { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + capture(lambdaSlot), + any(), + any() + ) + } + lambdaSlot.captured.invoke() + verify { activity.toast(R.string.no_reader_application_installed) } + } + + @Test + fun testFileDownloadingSuccessfull() { + val toastMessage = activity.getString(R.string.save_media_saved, savedFile.name) + mockkStatic(Toast::class) + justRun { + Toast.makeText( + activity, + toastMessage, + Toast.LENGTH_LONG + ).show() + } + val lambdaSlot = slot<() -> Unit>() + downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog( + demoUrl, + "application/pdf" + ) + verify { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + any(), + capture(lambdaSlot), + any() + ) + } + lambdaSlot.captured.invoke() + verify { activity.toast(toastMessage) } + } + + @Test + fun testUserClicksOnNoThanksButton() { + val lambdaSlot = slot<() -> Unit>() + downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + verify { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + any(), + any(), + capture(lambdaSlot) + ) + } + lambdaSlot.captured.invoke() + verify(exactly = 0) { activity.startActivity(any()) } + } + + @Test + fun testIfDownloadFailed() { + val downloadOrOpenEpubAndPdfHandler = DownloadOrOpenEpubAndPdfHandler( + activity, + sharedPreferenceUtil, + alertDialogShower, + zimReaderContainer + ) + mockkStatic(Toast::class) + justRun { + Toast.makeText(activity, R.string.save_media_error, Toast.LENGTH_LONG).show() + } + val lambdaSlot = slot<() -> Unit>() + downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(null, "application/pdf") + verify { + alertDialogShower.show( + KiwixDialog.DownloadOrOpenEpubAndPdf, + any(), + capture(lambdaSlot), + any() + ) + } + lambdaSlot.captured.invoke() + verify { activity.toast(R.string.save_media_error) } + } +} diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt index 230b4b000..80c9821db 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt @@ -36,8 +36,9 @@ class DownloadOrOpenEpubAndPdfHandler @Inject constructor( private val alertDialogShower: AlertDialogShower, private val zimReaderContainer: ZimReaderContainer ) { + var intent: Intent = Intent(Intent.ACTION_VIEW) - fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) { + fun showDownloadOrOpenEpubAndPdfDialog(url: String?, documentType: String?) { alertDialogShower.show( KiwixDialog.DownloadOrOpenEpubAndPdf, { openOrDownloadFile(url, documentType, true) }, @@ -46,7 +47,7 @@ class DownloadOrOpenEpubAndPdfHandler @Inject constructor( ) } - private fun openOrDownloadFile(url: String, documentType: String?, openFile: Boolean) { + private fun openOrDownloadFile(url: String?, documentType: String?, openFile: Boolean) { downloadFileFromUrl( url, null, @@ -72,7 +73,7 @@ class DownloadOrOpenEpubAndPdfHandler @Inject constructor( "${activity.packageName}.fileprovider", savedFile ) - val intent = Intent(Intent.ACTION_VIEW).apply { + intent.apply { setDataAndType(uri, documentType) flags = Intent.FLAG_ACTIVITY_NO_HISTORY addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) From c928943851071cf0ba6fe5e91acac13dd66f0c5e Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 20 Mar 2024 19:25:34 +0530 Subject: [PATCH 4/5] Fixed DownloadOrOpenEpubAndPdfHandlerTest failing on API level 24. --- .../kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt index 0a1dd8ad6..e3aa74970 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt @@ -28,7 +28,7 @@ import io.mockk.mockkStatic import io.mockk.slot import io.mockk.verify import org.junit.Before -import org.junit.Test +import org.junit.jupiter.api.Test import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer From 13fb613ebf2ab48e8b3d15db225ccd57adae5d4d Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 6 Jun 2024 14:09:31 +0530 Subject: [PATCH 5/5] Improved the naming of unsupported mimeType handler class. * Improved the dialog's message and title. --- ...t.kt => UnsupportedMimeTypeHandlerTest.kt} | 48 +++++++++---------- .../core/main/CoreReaderFragment.kt | 8 ++-- .../core/main/CoreWebViewClient.kt | 8 ++-- .../kiwixmobile/core/main/WebViewCallback.kt | 2 +- .../core/utils/dialog/KiwixDialog.kt | 8 ++-- ...ndler.kt => UnsupportedMimeTypeHandler.kt} | 12 ++--- core/src/main/res/values/strings.xml | 4 +- 7 files changed, 45 insertions(+), 45 deletions(-) rename app/src/androidTest/java/org/kiwix/kiwixmobile/utils/{DownloadOrOpenEpubAndPdfHandlerTest.kt => UnsupportedMimeTypeHandlerTest.kt} (74%) rename core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/{DownloadOrOpenEpubAndPdfHandler.kt => UnsupportedMimeTypeHandler.kt} (87%) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/UnsupportedMimeTypeHandlerTest.kt similarity index 74% rename from app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt rename to app/src/androidTest/java/org/kiwix/kiwixmobile/utils/UnsupportedMimeTypeHandlerTest.kt index e3aa74970..939481c38 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/DownloadOrOpenEpubAndPdfHandlerTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/utils/UnsupportedMimeTypeHandlerTest.kt @@ -34,12 +34,12 @@ import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower -import org.kiwix.kiwixmobile.core.utils.dialog.DownloadOrOpenEpubAndPdfHandler +import org.kiwix.kiwixmobile.core.utils.dialog.UnsupportedMimeTypeHandler import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog import java.io.File import java.io.InputStream -class DownloadOrOpenEpubAndPdfHandlerTest { +class UnsupportedMimeTypeHandlerTest { private val demoUrl = "content://demoPdf.pdf" private val demoFileName = "demoPdf.pdf" private val sharedPreferenceUtil: SharedPreferenceUtil = mockk() @@ -49,7 +49,7 @@ class DownloadOrOpenEpubAndPdfHandlerTest { private val activity: Activity = mockk() private val webResourceResponse: WebResourceResponse = mockk() private val inputStream: InputStream = mockk() - private val downloadOrOpenEpubAndPdfHandler = DownloadOrOpenEpubAndPdfHandler( + private val unsupportedMimeTypeHandler = UnsupportedMimeTypeHandler( activity, sharedPreferenceUtil, alertDialogShower, @@ -78,23 +78,23 @@ class DownloadOrOpenEpubAndPdfHandlerTest { } returns "Saved media as $demoFileName to Downloads/org.kiwix…/" every { savedFile.path } returns "Emulated/0/Downloads/$demoFileName" every { savedFile.exists() } returns true - downloadOrOpenEpubAndPdfHandler.intent = mockk() - every { downloadOrOpenEpubAndPdfHandler.intent.setDataAndType(any(), any()) } returns mockk() - every { downloadOrOpenEpubAndPdfHandler.intent.setFlags(any()) } returns mockk() - every { downloadOrOpenEpubAndPdfHandler.intent.addFlags(any()) } returns mockk() + unsupportedMimeTypeHandler.intent = mockk() + every { unsupportedMimeTypeHandler.intent.setDataAndType(any(), any()) } returns mockk() + every { unsupportedMimeTypeHandler.intent.setFlags(any()) } returns mockk() + every { unsupportedMimeTypeHandler.intent.addFlags(any()) } returns mockk() } @Test fun testOpeningFileInExternalReaderApplication() { every { - downloadOrOpenEpubAndPdfHandler.intent.resolveActivity(activity.packageManager) + unsupportedMimeTypeHandler.intent.resolveActivity(activity.packageManager) } returns mockk() - every { activity.startActivity(downloadOrOpenEpubAndPdfHandler.intent) } returns mockk() + every { activity.startActivity(unsupportedMimeTypeHandler.intent) } returns mockk() val lambdaSlot = slot<() -> Unit>() - downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + unsupportedMimeTypeHandler.showSaveOrOpenUnsupportedFilesDialog(demoUrl, "application/pdf") verify { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, + KiwixDialog.SaveOrOpenUnsupportedFiles, capture(lambdaSlot), any(), any() @@ -102,24 +102,24 @@ class DownloadOrOpenEpubAndPdfHandlerTest { } lambdaSlot.captured.invoke() verify { - activity.startActivity(downloadOrOpenEpubAndPdfHandler.intent) + activity.startActivity(unsupportedMimeTypeHandler.intent) } } @Test fun testOpeningFileWhenNoReaderApplicationInstalled() { every { - downloadOrOpenEpubAndPdfHandler.intent.resolveActivity(activity.packageManager) + unsupportedMimeTypeHandler.intent.resolveActivity(activity.packageManager) } returns null mockkStatic(Toast::class) justRun { Toast.makeText(activity, R.string.no_reader_application_installed, Toast.LENGTH_LONG).show() } val lambdaSlot = slot<() -> Unit>() - downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + unsupportedMimeTypeHandler.showSaveOrOpenUnsupportedFilesDialog(demoUrl, "application/pdf") verify { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, + KiwixDialog.SaveOrOpenUnsupportedFiles, capture(lambdaSlot), any(), any() @@ -130,7 +130,7 @@ class DownloadOrOpenEpubAndPdfHandlerTest { } @Test - fun testFileDownloadingSuccessfull() { + fun testFileSavedSuccessfully() { val toastMessage = activity.getString(R.string.save_media_saved, savedFile.name) mockkStatic(Toast::class) justRun { @@ -141,13 +141,13 @@ class DownloadOrOpenEpubAndPdfHandlerTest { ).show() } val lambdaSlot = slot<() -> Unit>() - downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog( + unsupportedMimeTypeHandler.showSaveOrOpenUnsupportedFilesDialog( demoUrl, "application/pdf" ) verify { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, + KiwixDialog.SaveOrOpenUnsupportedFiles, any(), capture(lambdaSlot), any() @@ -160,10 +160,10 @@ class DownloadOrOpenEpubAndPdfHandlerTest { @Test fun testUserClicksOnNoThanksButton() { val lambdaSlot = slot<() -> Unit>() - downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(demoUrl, "application/pdf") + unsupportedMimeTypeHandler.showSaveOrOpenUnsupportedFilesDialog(demoUrl, "application/pdf") verify { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, + KiwixDialog.SaveOrOpenUnsupportedFiles, any(), any(), capture(lambdaSlot) @@ -174,8 +174,8 @@ class DownloadOrOpenEpubAndPdfHandlerTest { } @Test - fun testIfDownloadFailed() { - val downloadOrOpenEpubAndPdfHandler = DownloadOrOpenEpubAndPdfHandler( + fun testIfSavingFailed() { + val downloadOrOpenEpubAndPdfHandler = UnsupportedMimeTypeHandler( activity, sharedPreferenceUtil, alertDialogShower, @@ -186,10 +186,10 @@ class DownloadOrOpenEpubAndPdfHandlerTest { Toast.makeText(activity, R.string.save_media_error, Toast.LENGTH_LONG).show() } val lambdaSlot = slot<() -> Unit>() - downloadOrOpenEpubAndPdfHandler.showDownloadOrOpenEpubAndPdfDialog(null, "application/pdf") + downloadOrOpenEpubAndPdfHandler.showSaveOrOpenUnsupportedFilesDialog(null, "application/pdf") verify { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, + KiwixDialog.SaveOrOpenUnsupportedFiles, any(), capture(lambdaSlot), any() diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 9a270156b..11249f14a 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -156,7 +156,7 @@ import org.kiwix.kiwixmobile.core.utils.TAG_FILE_SEARCHED_NEW_TAB import org.kiwix.kiwixmobile.core.utils.TAG_KIWIX import org.kiwix.kiwixmobile.core.utils.UpdateUtils.reformatProviderUrl import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower -import org.kiwix.kiwixmobile.core.utils.dialog.DownloadOrOpenEpubAndPdfHandler +import org.kiwix.kiwixmobile.core.utils.dialog.UnsupportedMimeTypeHandler import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog import org.kiwix.kiwixmobile.core.utils.files.FileUtils.deleteCachedFiles import org.kiwix.kiwixmobile.core.utils.files.FileUtils.readFile @@ -325,7 +325,7 @@ abstract class CoreReaderFragment : @JvmField @Inject - var downloadOrOpenEpubAndPdfHandler: DownloadOrOpenEpubAndPdfHandler? = null + var unsupportedMimeTypeHandler: UnsupportedMimeTypeHandler? = null private var hideBackToTopTimer: CountDownTimer? = null private var documentSections: MutableList? = null private var isBackToTopEnabled = false @@ -1547,8 +1547,8 @@ abstract class CoreReaderFragment : externalLinkOpener?.openExternalUrl(intent) } - override fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) { - downloadOrOpenEpubAndPdfHandler?.showDownloadOrOpenEpubAndPdfDialog(url, documentType) + override fun showSaveOrOpenUnsupportedFilesDialog(url: String, documentType: String?) { + unsupportedMimeTypeHandler?.showSaveOrOpenUnsupportedFilesDialog(url, documentType) } protected fun openZimFile( diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt index b49625355..25b8ed045 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreWebViewClient.kt @@ -44,14 +44,14 @@ open class CoreWebViewClient( url = convertLegacyUrl(url) urlWithAnchor = if (url.contains("#")) url else null if (zimReaderContainer.isRedirect(url)) { - if (handleEpubAndPdf(url)) { + if (handleUnsupportedFiles(url)) { return true } view.loadUrl(zimReaderContainer.getRedirect(url)) return true } if (url.startsWith(ZimFileReader.CONTENT_PREFIX)) { - return handleEpubAndPdf(url) + return handleUnsupportedFiles(url) } if (url.startsWith("javascript:")) { // Allow javascript for HTML functions and code execution (EX: night mode) @@ -77,10 +77,10 @@ open class CoreWebViewClient( } @Suppress("NestedBlockDepth") - private fun handleEpubAndPdf(url: String): Boolean { + private fun handleUnsupportedFiles(url: String): Boolean { val extension = MimeTypeMap.getFileExtensionFromUrl(url) if (DOCUMENT_TYPES.containsKey(extension)) { - callback.showDownloadOrOpenEpubAndPdfDialog(url, DOCUMENT_TYPES[extension]) + callback.showSaveOrOpenUnsupportedFilesDialog(url, DOCUMENT_TYPES[extension]) return true } return false diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt index 6b5ef57df..25c890250 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/WebViewCallback.kt @@ -25,7 +25,7 @@ interface WebViewCallback { fun webViewUrlFinishedLoading() fun webViewFailedLoading(failingUrl: String) fun openExternalUrl(intent: Intent) - fun showDownloadOrOpenEpubAndPdfDialog(url: String, documentType: String?) + fun showSaveOrOpenUnsupportedFilesDialog(url: String, documentType: String?) fun webViewProgressChanged(progress: Int, webView: WebView) fun webViewTitleUpdated(title: String) fun webViewPageChanged(page: Int, maxPages: Int) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt index e00015192..3264eec6d 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt @@ -108,11 +108,11 @@ sealed class KiwixDialog( cancelable = false ) - object DownloadOrOpenEpubAndPdf : KiwixDialog( - R.string.download_or_open_pdf_and_epub_content_dialog_title, - R.string.download_or_open_pdf_and_epub_content_dialog_message, + object SaveOrOpenUnsupportedFiles : KiwixDialog( + R.string.save_or_open_unsupported_files_dialog_title, + R.string.save_or_open_unsupported_files_dialog_message, R.string.open, - R.string.download, + R.string.save, neutralMessage = R.string.no_thanks ) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/UnsupportedMimeTypeHandler.kt similarity index 87% rename from core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt rename to core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/UnsupportedMimeTypeHandler.kt index 80c9821db..830a94f58 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/DownloadOrOpenEpubAndPdfHandler.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/UnsupportedMimeTypeHandler.kt @@ -30,7 +30,7 @@ import org.kiwix.kiwixmobile.core.utils.files.FileUtils.downloadFileFromUrl import java.io.File import javax.inject.Inject -class DownloadOrOpenEpubAndPdfHandler @Inject constructor( +class UnsupportedMimeTypeHandler @Inject constructor( private val activity: Activity, private val sharedPreferenceUtil: SharedPreferenceUtil, private val alertDialogShower: AlertDialogShower, @@ -38,16 +38,16 @@ class DownloadOrOpenEpubAndPdfHandler @Inject constructor( ) { var intent: Intent = Intent(Intent.ACTION_VIEW) - fun showDownloadOrOpenEpubAndPdfDialog(url: String?, documentType: String?) { + fun showSaveOrOpenUnsupportedFilesDialog(url: String?, documentType: String?) { alertDialogShower.show( - KiwixDialog.DownloadOrOpenEpubAndPdf, - { openOrDownloadFile(url, documentType, true) }, - { openOrDownloadFile(url, documentType, false) }, + KiwixDialog.SaveOrOpenUnsupportedFiles, + { openOrSaveFile(url, documentType, true) }, + { openOrSaveFile(url, documentType, false) }, { } ) } - private fun openOrDownloadFile(url: String?, documentType: String?, openFile: Boolean) { + private fun openOrSaveFile(url: String?, documentType: String?, openFile: Boolean) { downloadFileFromUrl( url, null, diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml index 2df474630..94034e08d 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -373,6 +373,6 @@ ZIM file which has the reading content Selecting this language will prioritize displaying downloadable books in that language at the top. Go to previous screen - Download or Open this (Epub/Pdf) file? - Choosing Open will open this file in your external Epub/PDF reader application. + Save or Open this file? + Choosing Open will open this file in external reader application.