From 1791911f136c51abb25de25df951696ccfb86ec4 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 6 Jan 2025 15:58:37 +0530 Subject: [PATCH 1/3] Fixed: infinite loop and improve error handling for fetching random article. * Fixed an issue where the app would enter an infinite loop when the zimFileReader was null, causing a crash. * Improved the condition to check if zimFileReader is null and show a proper error message to the user without retrying. * Added a retry mechanism to attempt fetching the random article twice in case of an internal error in libzim. * After two failed attempts, an error message is displayed to the user if the random article is not found. --- .../core/main/CoreReaderFragment.kt | 35 ++++++++++++++++--- core/src/main/res/values/strings.xml | 2 ++ 2 files changed, 33 insertions(+), 4 deletions(-) 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 5a353ce2f..a2edbc7f8 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 @@ -109,6 +109,7 @@ import org.kiwix.kiwixmobile.core.base.BaseFragment import org.kiwix.kiwixmobile.core.base.FragmentActivityExtensions import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks import org.kiwix.kiwixmobile.core.databinding.FragmentReaderBinding +import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.consumeObservable import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.hasNotificationPermission import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isLandScapeMode @@ -2173,13 +2174,39 @@ abstract class CoreReaderFragment : } } - private fun openRandomArticle() { + /** + * Attempts to open a random article from the ZIM file. If the article URL cannot be retrieved + * due to internal errors or a missing ZIM file reader, the method will retry up to a certain + * number of times (default: 2). If the article URL is still unavailable after retries, + * an error message will be displayed to the user. The method ensures that the user does not + * see a blank or previously loaded page, but instead receives an appropriate error message + * if the random article cannot be fetched. + * + * @param retryCount The number of attempts left to retry fetching the random article. + * Default is 2. The method decreases this count with each retry attempt. + */ + private fun openRandomArticle(retryCount: Int = 2) { + // Check if the ZIM file reader is available, if not show an error and exit. + if (zimReaderContainer?.zimFileReader == null) { + toast(R.string.error_loading_random_article_zim_not_loaded) + return + } val articleUrl = zimReaderContainer?.getRandomArticleUrl() if (articleUrl == null) { // Check if the random url is null due to some internal error in libzim(See #3926) - // then again try to get the random article. So that the user can see the random article - // instead of a (blank/same page) currently loaded in the webView. - openRandomArticle() + // then try one more time to get the random article. So that the user can see the + // random article instead of a (blank/same page) currently loaded in the webView. + if (retryCount > ZERO) { + Log.e( + TAG_KIWIX, + "Random article URL is null, retrying... Remaining attempts: $retryCount" + ) + openRandomArticle(retryCount - 1) + } else { + // if it is failed to find the random article two times then show a error to user. + Log.e(TAG_KIWIX, "Failed to load random article after multiple attempts") + toast(R.string.could_not_find_random_article) + } return } Log.d(TAG_KIWIX, "openRandomArticle: $articleUrl") diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml index 995cc2e12..4d1062ace 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -55,6 +55,8 @@ Unable to open ZIM file Error: The selected file is not a valid ZIM file. Error: Loading article (Url: %1$s) failed. + Unable to load article. The ZIM file is not properly loaded. + Unable to find a random article. Please try again later. Display Information Version From c60ea98b73dbc6afc4a912df60693e4b19da5ce4 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 8 Jan 2025 15:35:47 +0530 Subject: [PATCH 2/3] Fixed the issue where opening a random article caused the application to crash. * The issue occurred in the Play Store variant, specifically with the copy/move functionality for ZIM files. After copying or moving a ZIM file, the application attempts to open it in the reader. However, if the application tries to open a split ZIM file with only one chunk, it throws an error indicating that the file is not a valid ZIM file (which is expected, as not all ZIM chunks have been copied/moved). When this error occurs, the menu buttons remain enabled, allowing the user to interact with them. If the user then attempts to open a random article, the application enters an infinite loop and crashes. To resolve this, we now disable the menu controls whenever an error occurs while opening a ZIM file, preventing the user from using them in such scenarios. --- .../org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt | 4 ++++ 1 file changed, 4 insertions(+) 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 a2edbc7f8..26ca13f03 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 @@ -1735,6 +1735,10 @@ abstract class CoreReaderFragment : mainMenu?.onFileOpened(urlIsValid()) setUpBookmarks(zimFileReader) } ?: kotlin.run { + // If the ZIM file is not opened properly (especially for ZIM chunks), exit the book to + // disable all controls for this ZIM file. This prevents potential crashes. + // See issue #4161 for more details. + exitBook() requireActivity().toast(R.string.error_file_invalid, Toast.LENGTH_LONG) } } From 85427c45513802ccd960627a12aeb3b5e89b65d2 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Sat, 18 Jan 2025 13:57:53 +0530 Subject: [PATCH 3/3] Improved the behavior of enabling menu buttons. * Previously, when a ZIM file was in the process of opening, the reopenBook method was used to hide the "Open Library" button if it was visible. However, this method also enabled the menu buttons. If an error occurred while loading the ZIM file, the controls remained enabled, which was not desirable. Now, the "Open Library" button is hidden, and the menu buttons are shown only after the ZIM file has successfully loaded. If an error occurs during loading, an error message is displayed (as it was previously), and the UI is updated accordingly. --- .../java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 26ca13f03..26c345ca1 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 @@ -1691,7 +1691,8 @@ abstract class CoreReaderFragment : if (zimReaderSource.canOpenInLibkiwix()) { // Show content if there is `Open Library` button showing // and we are opening the ZIM file - reopenBook() + hideNoBookOpenViews() + contentFrame?.visibility = View.VISIBLE openAndSetInContainer(zimReaderSource) updateTitle() } else {