From 1ed02e5ab08ac53c41d6e5d21beeca9412de680c Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 30 Jan 2025 17:25:58 +0530 Subject: [PATCH 1/5] Fixed: FIND_IN_PAGE feature only works with first tab page. * The previously loaded article was not loading in custom apps when switching to another fragment and then returning to the reader screen. Instead, the home page of the ZIM file was loading. A fix has been implemented to resolve this issue. --- .../kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt index 362bdc69b..a5b5730ad 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt @@ -144,7 +144,6 @@ class CustomReaderFragment : CoreReaderFragment() { zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) } else { openObbOrZim() - manageExternalLaunchAndRestoringViewState() } requireArguments().clear() } @@ -207,11 +206,15 @@ class CustomReaderFragment : CoreReaderFragment() { val bookOnDisk = BookOnDisk(zimFileReader) repositoryActions?.saveBook(bookOnDisk) } + // Open the previous loaded pages after ZIM file loads. + manageExternalLaunchAndRestoringViewState() } is ValidationState.HasBothFiles -> { it.zimFile.delete() openZimFile(ZimReaderSource(it.obbFile), true) + // Open the previous loaded pages after ZIM file loads. + manageExternalLaunchAndRestoringViewState() } else -> {} From 5e05fe81d980c49407be7ac561469c57302629f5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 30 Jan 2025 18:37:24 +0530 Subject: [PATCH 2/5] Added test cases for custom apps to cover this scenario and prevent future errors. * Refactored some deprecated methods used in test cases. --- .../search/SearchFragmentTestForCustomApp.kt | 73 +++++++++++++++++-- .../kiwixmobile/custom/search/SearchRobot.kt | 51 +++++++++++++ .../kiwixmobile/custom/testutils/TestUtils.kt | 4 +- 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt index 0dcc83daf..8393a86d1 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt @@ -90,8 +90,10 @@ class SearchFragmentTestForCustomApp { private lateinit var downloadingZimFile: File private lateinit var activityScenario: ActivityScenario - private val rayCharlesZimFileUrl = + private val scientificAllianceZIMUrl = "https://download.kiwix.org/zim/zimit/scientific-alliance.obscurative.ru_ru_all_2024-06.zim" + private val rayCharlesZIMFileUrl = + "https://dev.kiwix.org/kiwix-android/test/wikipedia_en_ray_charles_maxi_2023-12.zim" @Before fun waitForIdle() { @@ -258,6 +260,44 @@ class SearchFragmentTestForCustomApp { } } + @Test + fun testPreviouslyLoadedArticleLoadsAgainWhenSwitchingToAnotherScreen() { + activityScenario.onActivity { + customMainActivity = it + } + // test with a large ZIM file to properly test the scenario + downloadingZimFile = getDownloadingZimFileFromDataFolder() + getOkkHttpClientForTesting().newCall(downloadRequest(rayCharlesZIMFileUrl)).execute() + .use { response -> + if (response.isSuccessful) { + response.body?.let { responseBody -> + writeZimFileData(responseBody, downloadingZimFile) + } + } else { + throw RuntimeException( + "Download Failed. Error: ${response.message}\n" + + " Status Code: ${response.code}" + ) + } + } + UiThreadStatement.runOnUiThread { + customMainActivity.navigate(customMainActivity.readerFragmentResId) + } + openZimFileInReader(zimFile = downloadingZimFile) + search { + // click on home button to load the main page of ZIM file. + clickOnHomeButton() + // click on an article to load the other page. + clickOnAFoolForYouArticle() + assertAFoolForYouArticleLoaded() + // open note screen. + openNoteFragment() + pressBack() + // after came back check the previously loaded article is still showing or not. + assertAFoolForYouArticleLoaded() + } + } + private fun openSearchWithQuery(query: String = "") { UiThreadStatement.runOnUiThread { customMainActivity.openSearch(searchString = query) @@ -270,7 +310,10 @@ class SearchFragmentTestForCustomApp { } } - private fun openZimFileInReader(assetFileDescriptor: AssetFileDescriptor) { + private fun openZimFileInReader( + assetFileDescriptor: AssetFileDescriptor? = null, + zimFile: File? = null + ) { UiThreadStatement.runOnUiThread { val navHostFragment: NavHostFragment = customMainActivity.supportFragmentManager @@ -280,10 +323,17 @@ class SearchFragmentTestForCustomApp { val customReaderFragment = navHostFragment.childFragmentManager.fragments[0] as CustomReaderFragment runBlocking { - customReaderFragment.openZimFile( - ZimReaderSource(null, null, listOf(assetFileDescriptor)), - true - ) + assetFileDescriptor?.let { + customReaderFragment.openZimFile( + ZimReaderSource(assetFileDescriptorList = listOf(assetFileDescriptor)), + true + ) + } ?: run { + customReaderFragment.openZimFile( + ZimReaderSource(zimFile), + true + ) + } } } } @@ -318,9 +368,9 @@ class SearchFragmentTestForCustomApp { } } - private fun downloadRequest() = + private fun downloadRequest(zimUrl: String = scientificAllianceZIMUrl) = Request.Builder() - .url(URI.create(rayCharlesZimFileUrl).toURL()) + .url(URI.create(zimUrl).toURL()) .build() private fun getDownloadingZimFile(): File { @@ -330,6 +380,13 @@ class SearchFragmentTestForCustomApp { return zimFile } + private fun getDownloadingZimFileFromDataFolder(): File { + val zimFile = File(context.getExternalFilesDirs(null)[0], "ray_charles.zim") + if (zimFile.exists()) zimFile.delete() + zimFile.createNewFile() + return zimFile + } + @Singleton private fun getOkkHttpClientForTesting(): OkHttpClient = OkHttpClient().newBuilder() diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt index 3191c6a8e..5b55c26dd 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt @@ -19,19 +19,31 @@ package org.kiwix.kiwixmobile.custom.search import android.view.KeyEvent +import androidx.core.view.GravityCompat import androidx.recyclerview.widget.RecyclerView import androidx.test.espresso.Espresso +import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions +import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions import androidx.test.espresso.contrib.RecyclerViewActions import androidx.test.espresso.matcher.ViewMatchers +import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.espresso.web.assertion.WebViewAssertions.webMatches import androidx.test.espresso.web.sugar.Web +import androidx.test.espresso.web.sugar.Web.onWebView import androidx.test.espresso.web.webdriver.DriverAtoms +import androidx.test.espresso.web.webdriver.DriverAtoms.findElement +import androidx.test.espresso.web.webdriver.DriverAtoms.getText +import androidx.test.espresso.web.webdriver.DriverAtoms.webClick import androidx.test.espresso.web.webdriver.Locator import androidx.test.uiautomator.UiDevice +import com.adevinta.android.barista.interaction.BaristaDrawerInteractions.openDrawerWithGravity import com.adevinta.android.barista.interaction.BaristaSleepInteractions import com.adevinta.android.barista.internal.matcher.HelperMatchers +import org.hamcrest.CoreMatchers.containsString import org.kiwix.kiwixmobile.core.R +import org.kiwix.kiwixmobile.custom.R.id import org.kiwix.kiwixmobile.custom.testutils.TestUtils import org.kiwix.kiwixmobile.custom.testutils.TestUtils.testFlakyView @@ -118,4 +130,43 @@ class SearchRobot { ) }) } + + fun clickOnHomeButton() { + testFlakyView({ + Espresso.onView(ViewMatchers.withId(R.id.bottom_toolbar_home)) + .perform(ViewActions.click()) + }) + } + + fun clickOnAFoolForYouArticle() { + BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS.toLong()) + testFlakyView({ + onWebView() + .withElement( + findElement( + Locator.XPATH, + "//*[contains(text(), 'A Fool for You')]" + ) + ).perform(webClick()) + }) + } + + fun assertAFoolForYouArticleLoaded() { + BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS.toLong()) + testFlakyView({ + onWebView() + .withElement( + findElement( + Locator.XPATH, + "//*[contains(text(), '\"A Fool for You\"')]" + ) + ).check(webMatches(getText(), containsString("\"A Fool for You\""))) + }) + } + + fun openNoteFragment() { + BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS.toLong()) + openDrawerWithGravity(id.custom_drawer_container, GravityCompat.START) + testFlakyView({ onView(withText(R.string.pref_notes)).perform(click()) }) + } } diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/testutils/TestUtils.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/testutils/TestUtils.kt index af59b9272..238368cb5 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/testutils/TestUtils.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/testutils/TestUtils.kt @@ -21,7 +21,6 @@ package org.kiwix.kiwixmobile.custom.testutils import android.content.Context import android.content.ContextWrapper import android.content.Intent -import androidx.core.content.ContextCompat import androidx.test.uiautomator.By import androidx.test.uiautomator.UiDevice import androidx.test.uiautomator.UiObject @@ -32,6 +31,7 @@ import java.io.File object TestUtils { private const val TAG = "TESTUTILS" var TEST_PAUSE_MS_FOR_SEARCH_TEST = 1000 + var TEST_PAUSE_MS = 3000 @JvmStatic fun isSystemUINotRespondingDialogVisible(uiDevice: UiDevice) = @@ -92,7 +92,7 @@ object TestUtils { @JvmStatic fun deleteTemporaryFilesOfTestCases(context: Context) { - ContextCompat.getExternalFilesDirs(context, null).filterNotNull() + context.getExternalFilesDirs(null).filterNotNull() .map(::deleteAllFilesInDirectory) ContextWrapper(context).externalMediaDirs.filterNotNull() .map(::deleteAllFilesInDirectory) From 6604875bde0063790f21b6f3829249eea94ef1d7 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 30 Jan 2025 18:39:56 +0530 Subject: [PATCH 3/5] Refactored the deprecated method used in app module test cases. --- .../java/org/kiwix/kiwixmobile/testutils/TestUtils.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt index c6c55613c..a525a954d 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt @@ -220,7 +220,7 @@ object TestUtils { @JvmStatic fun deleteTemporaryFilesOfTestCases(context: Context) { - ContextCompat.getExternalFilesDirs(context, null).filterNotNull() + context.getExternalFilesDirs(null).filterNotNull() .map(::deleteAllFilesInDirectory) ContextWrapper(context).externalMediaDirs.filterNotNull() .map(::deleteAllFilesInDirectory) From 499ea74048d8952e64b90f6e1b047d55c791c2d5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 31 Jan 2025 14:17:10 +0530 Subject: [PATCH 4/5] Fixed: `FIND_IN_PAGE` not working in tabs other than the first one. * Improved the logic for handling search-related operations to ensure they are performed after tabs are fully restored. * Previously, the search observation was initiated before the tabs were restored, causing the `FIND_IN_PAGE` feature to only work in the first tab. This happened because the webView list was empty at the time of observation. * With the ZIM file opening and tab restoration now moved to background threads, the search observation logic has been moved to after the tabs are restored. This ensures that all webViews are properly initialized before observing search actions. * This approach is more robust and aligns with the correct lifecycle of tab restoration and search functionality. --- .../core/main/CoreReaderFragment.kt | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 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 e260d99fe..6ad2b4cef 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 @@ -505,16 +505,6 @@ abstract class CoreReaderFragment : readAloudService?.registerCallBack(this@CoreReaderFragment) } } - requireActivity().observeNavigationResult( - FIND_IN_PAGE_SEARCH_STRING, - viewLifecycleOwner, - Observer(::findInPage) - ) - requireActivity().observeNavigationResult( - TAG_FILE_SEARCHED, - viewLifecycleOwner, - Observer(::openSearchItem) - ) handleClicks() } @@ -2617,6 +2607,28 @@ abstract class CoreReaderFragment : Log.w(TAG_KIWIX, "Kiwix shared preferences corrupted", e) activity.toast(R.string.could_not_restore_tabs, Toast.LENGTH_LONG) } + // After restoring the tabs, observe any search actions that the user might have triggered. + // Since the ZIM file opening functionality has been moved to a background thread, + // we ensure that all necessary actions are completed before observing these search actions. + observeSearchActions() + } + + /** + * Observes any search-related actions triggered by the user, such as "Find in Page" or + * opening a specific search item. + * This method sets up observers for navigation results related to search functionality. + */ + private fun observeSearchActions() { + requireActivity().observeNavigationResult( + FIND_IN_PAGE_SEARCH_STRING, + viewLifecycleOwner, + Observer(::findInPage) + ) + requireActivity().observeNavigationResult( + TAG_FILE_SEARCHED, + viewLifecycleOwner, + Observer(::openSearchItem) + ) } override fun onReadAloudPauseOrResume(isPauseTTS: Boolean) { From a9364d5881747076463a86bdf010999019b765a6 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 31 Jan 2025 15:23:35 +0530 Subject: [PATCH 5/5] Fixed: The `SearchFragmentTestForCustomApp` was failing sometimes. --- .../custom/search/SearchFragmentTestForCustomApp.kt | 4 ++-- .../java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt index 8393a86d1..c0307a1ce 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt @@ -131,7 +131,7 @@ class SearchFragmentTestForCustomApp { customMainActivity = it } // test with a large ZIM file to properly test the scenario - downloadingZimFile = getDownloadingZimFile() + downloadingZimFile = getDownloadingZimFileFromDataFolder() getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> @@ -147,7 +147,7 @@ class SearchFragmentTestForCustomApp { UiThreadStatement.runOnUiThread { customMainActivity.navigate(customMainActivity.readerFragmentResId) } - openZimFileInReaderWithAssetFileDescriptor(downloadingZimFile) + openZimFileInReader(zimFile = downloadingZimFile) openSearchWithQuery() val searchTerm = "gard" val searchedItem = "Gardanta Spirito" diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt index 5b55c26dd..a266d0d07 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchRobot.kt @@ -100,14 +100,16 @@ class SearchRobot { } fun searchAndClickOnArticle(searchString: String) { - // wait a bit to properly load the ZIM file in the reader + // Wait a bit to properly load the ZIM file in the reader. BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS_FOR_SEARCH_TEST.toLong()) openSearchScreen() + // Wait a bit to properly visible the search screen. + BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS_FOR_SEARCH_TEST.toLong()) searchWithFrequentlyTypedWords(searchString) clickOnSearchItemInSearchList() } - fun clickOnSearchItemInSearchList() { + private fun clickOnSearchItemInSearchList() { testFlakyView({ BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS_FOR_SEARCH_TEST.toLong()) Espresso.onView(ViewMatchers.withId(R.id.search_list)).perform(