Merge pull request #4393 from kiwix/Fixes#4392

Fixed: Tabs not restoring in multiple navigation scenarios.
This commit is contained in:
Kelson 2025-08-20 16:05:13 +02:00 committed by GitHub
commit 8462128b70
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 190 additions and 61 deletions

View File

@ -46,11 +46,13 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.kiwix.kiwixmobile.BaseActivityTest
import org.kiwix.kiwixmobile.core.main.CoreMainActivity
import org.kiwix.kiwixmobile.core.utils.LanguageUtils.Companion.handleLocaleChange
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.core.utils.TestingUtils.COMPOSE_TEST_RULE_ORDER
import org.kiwix.kiwixmobile.core.utils.TestingUtils.RETRY_RULE_ORDER
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.page.bookmarks.bookmarks
import org.kiwix.kiwixmobile.testutils.RetryRule
import org.kiwix.kiwixmobile.testutils.TestUtils
import org.kiwix.kiwixmobile.testutils.TestUtils.closeSystemDialogs
@ -115,6 +117,56 @@ class KiwixReaderFragmentTest : BaseActivityTest() {
}
}
@Test
fun testTabsRestoredAfterNavigatingLeftDrawerScreens() {
activityScenario.onActivity {
kiwixMainActivity = it
kiwixMainActivity.navigate(KiwixDestination.Library.route)
}
composeTestRule.waitForIdle()
val zimFile = getLocalZIMFile()
openKiwixReaderFragmentWithFile(zimFile)
reader {
checkZimFileLoadedSuccessful(composeTestRule)
// open a new tab
openSearchWithQuery("Android", zimFile)
openAndroidArticleInNewTab(composeTestRule)
checkZimFileLoadedSuccessful(composeTestRule)
// open bookmark screen.
bookmarks {
openBookmarkScreen(kiwixMainActivity as CoreMainActivity, composeTestRule)
assertBookMarksDisplayed(composeTestRule)
}
composeTestRule.waitForIdle()
pressBack()
assertTabsRestored(composeTestRule)
}
}
@Test
fun testTabsRestoredWhenNavigatingToOtherScreenViaBottomAppBar() {
activityScenario.onActivity {
kiwixMainActivity = it
kiwixMainActivity.navigate(KiwixDestination.Library.route)
}
composeTestRule.waitForIdle()
val zimFile = getLocalZIMFile()
openKiwixReaderFragmentWithFile(zimFile)
reader {
checkZimFileLoadedSuccessful(composeTestRule)
// open a new tab
openSearchWithQuery("Android", zimFile)
openAndroidArticleInNewTab(composeTestRule)
checkZimFileLoadedSuccessful(composeTestRule)
// open local library screen.
openLocalLibraryScreenViaBottomAppBar(composeTestRule)
composeTestRule.waitForIdle()
// press back to come back to reader screen.
pressBack()
assertTabsRestored(composeTestRule)
}
}
@Test
fun testTabClosedDialog() {
activityScenario.onActivity {
@ -122,26 +174,7 @@ class KiwixReaderFragmentTest : BaseActivityTest() {
kiwixMainActivity.navigate(KiwixDestination.Library.route)
}
composeTestRule.waitForIdle()
val loadFileStream =
KiwixReaderFragmentTest::class.java.classLoader.getResourceAsStream("testzim.zim")
val zimFile =
File(
context.getExternalFilesDirs(null)[0],
"testzim.zim"
)
if (zimFile.exists()) zimFile.delete()
zimFile.createNewFile()
loadFileStream.use { inputStream ->
val outputStream: OutputStream = FileOutputStream(zimFile)
outputStream.use { it ->
val buffer = ByteArray(inputStream.available())
var length: Int
while (inputStream.read(buffer).also { length = it } > 0) {
it.write(buffer, 0, length)
}
}
}
openKiwixReaderFragmentWithFile(zimFile)
openKiwixReaderFragmentWithFile(getLocalZIMFile())
composeTestRule.waitForIdle()
reader {
checkZimFileLoadedSuccessful(composeTestRule)
@ -263,6 +296,43 @@ class KiwixReaderFragmentTest : BaseActivityTest() {
}
}
private fun getLocalZIMFile(): File {
val loadFileStream =
KiwixReaderFragmentTest::class.java.classLoader.getResourceAsStream("testzim.zim")
val zimFile =
File(
context.getExternalFilesDirs(null)[0],
"testzim.zim"
)
if (zimFile.exists()) zimFile.delete()
zimFile.createNewFile()
loadFileStream.use { inputStream ->
val outputStream: OutputStream = FileOutputStream(zimFile)
outputStream.use { it ->
val buffer = ByteArray(inputStream.available())
var length: Int
while (inputStream.read(buffer).also { length = it } > 0) {
it.write(buffer, 0, length)
}
}
}
return zimFile
}
private fun openSearchWithQuery(query: String = "", zimFile: File) {
UiThreadStatement.runOnUiThread {
if (zimFile.canRead()) {
kiwixMainActivity.openSearch(searchString = query)
} else {
throw RuntimeException(
"File $zimFile is not readable." +
" Original File $zimFile is readable = ${zimFile.canRead()}" +
" Size ${zimFile.length()}"
)
}
}
}
private fun openKiwixReaderFragmentWithFile(zimFile: File) {
UiThreadStatement.runOnUiThread {
val navOptions = NavOptions.Builder()

View File

@ -35,6 +35,9 @@ import org.kiwix.kiwixmobile.core.main.reader.CLOSE_ALL_TABS_BUTTON_TESTING_TAG
import org.kiwix.kiwixmobile.core.main.reader.READER_SCREEN_TESTING_TAG
import org.kiwix.kiwixmobile.core.main.reader.TAB_MENU_ITEM_TESTING_TAG
import org.kiwix.kiwixmobile.core.main.reader.TAB_TITLE_TESTING_TAG
import org.kiwix.kiwixmobile.core.search.OPEN_ITEM_IN_NEW_TAB_ICON_TESTING_TAG
import org.kiwix.kiwixmobile.main.BOTTOM_NAV_LIBRARY_ITEM_TESTING_TAG
import org.kiwix.kiwixmobile.core.main.reader.TABS_SIZE_TEXT_TESTING_TAG
import org.kiwix.kiwixmobile.testutils.TestUtils
import org.kiwix.kiwixmobile.testutils.TestUtils.testFlakyView
import org.kiwix.kiwixmobile.testutils.TestUtils.waitUntilTimeout
@ -115,4 +118,31 @@ class ReaderRobot : BaseRobot() {
)
})
}
fun openAndroidArticleInNewTab(composeTestRule: ComposeContentTestRule) {
testFlakyView({
composeTestRule.apply {
waitForIdle()
onAllNodesWithTag(OPEN_ITEM_IN_NEW_TAB_ICON_TESTING_TAG)[0].performClick()
}
})
}
fun assertTabsRestored(composeTestRule: ComposeContentTestRule) {
testFlakyView({
composeTestRule.apply {
waitForIdle()
onNodeWithTag(TABS_SIZE_TEXT_TESTING_TAG, useUnmergedTree = true).assertTextEquals("2")
}
})
}
fun openLocalLibraryScreenViaBottomAppBar(composeTestRule: ComposeContentTestRule) {
testFlakyView({
composeTestRule.apply {
waitForIdle()
onNodeWithTag(BOTTOM_NAV_LIBRARY_ITEM_TESTING_TAG).performClick()
}
})
}
}

View File

@ -272,10 +272,7 @@ class KiwixMainActivity : CoreMainActivity() {
if (intent?.action == ACTION_GET_CONTENT) {
navigate(KiwixDestination.Downloads.route) {
launchSingleTop = true
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
}
restoreState = true
popUpTo(navController.graph.findStartDestination().id)
}
}
}
@ -372,6 +369,9 @@ class KiwixMainActivity : CoreMainActivity() {
}
override fun openSearch(searchString: String, isOpenedFromTabView: Boolean, isVoice: Boolean) {
// remove the previous backStack entry with old arguments. Bug Fix #4392
removeArgumentsOfReaderScreen()
// Freshly open the search fragment.
navigate(
KiwixDestination.Search.createRoute(
searchString = searchString,
@ -414,6 +414,25 @@ class KiwixMainActivity : CoreMainActivity() {
shouldShowBottomAppBar.update { true }
}
/**
* Handles navigation from the left drawer to the Reader screen.
*
* Clears any existing Reader back stack entry (with its arguments)
* and replaces it with a fresh Reader screen using default arguments.
* This ensures old arguments are not retained when navigating
* via the left drawer.
*/
override fun removeArgumentsOfReaderScreen() {
if (navController.currentDestination?.route?.startsWith(readerFragmentRoute) == true) {
navigate(
readerFragmentRoute,
NavOptions.Builder()
.setPopUpTo(KiwixDestination.Reader.route, inclusive = true)
.build()
)
}
}
// Outdated shortcut ids(new_tab, get_content)
// Remove if the application has the outdated shortcuts.
private fun removeOutdatedIdShortcuts() {

View File

@ -198,11 +198,12 @@ fun BottomNavigationBar(
// Pop up to the start destination of the graph to avoid building up a large stack
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
// Bug fix #4392
saveState = item.route != KiwixDestination.Reader.route
}
// Restore state when reselecting a previously selected tab
restoreState = true
restoreState = item.route != KiwixDestination.Reader.route
}
}
},

View File

@ -47,7 +47,7 @@ data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.Bo
)
} else {
val navOptions = NavOptions.Builder()
.setPopUpTo(KiwixDestination.Library.route, inclusive = false)
.setPopUpTo(KiwixDestination.Reader.route, inclusive = true)
.build()
activity.navigate(
KiwixDestination.Reader.createRoute(zimFileUri = zimReaderSource.toDatabase()),

View File

@ -315,8 +315,8 @@ abstract class CoreMainActivity : BaseActivity(), WebViewProvider {
}
protected fun openHelpFragment() {
navigate(helpFragmentRoute)
handleDrawerOnNavigation()
navigate(helpFragmentRoute)
}
fun navigationDrawerIsOpen(): Boolean = leftDrawerState.isOpen
@ -403,6 +403,7 @@ abstract class CoreMainActivity : BaseActivity(), WebViewProvider {
protected fun handleDrawerOnNavigation() {
closeNavigationDrawer()
disableLeftDrawer()
removeArgumentsOfReaderScreen()
}
private fun setMainActivityToCoreApp() {
@ -508,4 +509,5 @@ abstract class CoreMainActivity : BaseActivity(), WebViewProvider {
abstract fun createApplicationShortcuts()
abstract fun hideBottomAppBar()
abstract fun showBottomAppBar()
abstract fun removeArgumentsOfReaderScreen()
}

View File

@ -1075,6 +1075,7 @@ abstract class CoreReaderFragment :
override fun onDestroyView() {
super.onDestroyView()
libkiwixBook = null
findInPageTitle = null
searchItemToOpen = null
pendingIntent = null

View File

@ -35,6 +35,7 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.style.TextOverflow
import org.kiwix.kiwixmobile.core.R
@ -57,6 +58,7 @@ const val TAKE_NOTE_MENU_ITEM_TESTING_TAG = "takeNoteMenuItemTestingTag"
const val FULL_SCREEN_MENU_ITEM_TESTING_TAG = "fullScreenMenuItemTestingTag"
const val RANDOM_ARTICLE_MENU_ITEM_TESTING_TAG = "randomArticleMenuItemTestingTag"
const val TAB_MENU_ITEM_TESTING_TAG = "tabMenuItemTestingTag"
const val TABS_SIZE_TEXT_TESTING_TAG = "tabsSizeTextTestingTag"
@Stable
class ReaderMenuState(
@ -227,7 +229,8 @@ class ReaderMenuState(
fontWeight = FontWeight.Bold,
fontSize = TAB_SWITCHER_TEXT_SIZE,
maxLines = 1,
overflow = TextOverflow.Ellipsis
overflow = TextOverflow.Ellipsis,
modifier = Modifier.testTag(TABS_SIZE_TEXT_TESTING_TAG)
)
}
}

View File

@ -50,6 +50,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.platform.LocalLayoutDirection
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.semantics
@ -73,6 +74,7 @@ const val SEARCH_FIELD_TESTING_TAG = "searchFieldTestingTag"
const val NO_SEARCH_RESULT_TESTING_TAG = "noSearchResultTestingTag"
const val FIND_IN_PAGE_TESTING_TAG = "findInPageTestingTag"
const val SEARCH_ITEM_TESTING_TAG = "searchItemTestingTag"
const val OPEN_ITEM_IN_NEW_TAB_ICON_TESTING_TAG = "openItemInNewTagIconTestingTag"
const val LOADING_ITEMS_BEFORE = 3
@OptIn(ExperimentalMaterial3Api::class)
@ -232,6 +234,7 @@ private fun SearchListItem(
IconButton(
onClick = { onNewTabIconClick(searchListItem) },
modifier = Modifier.testTag(OPEN_ITEM_IN_NEW_TAB_ICON_TESTING_TAG)
) {
Icon(
painter = painterResource(id = R.drawable.ic_open_in_new_24dp),

View File

@ -20,25 +20,14 @@ package org.kiwix.kiwixmobile.core.search.viewmodel.effects
import androidx.appcompat.app.AppCompatActivity
import org.kiwix.kiwixmobile.core.base.SideEffect
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResultOnCurrent
import org.kiwix.kiwixmobile.core.main.CoreMainActivity
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.popNavigationBackstack
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResult
import org.kiwix.kiwixmobile.core.main.FIND_IN_PAGE_SEARCH_STRING
data class SearchInPreviousScreen(private val searchString: String) : SideEffect<Unit> {
override fun invokeWith(activity: AppCompatActivity) {
val coreMainActivity = activity as CoreMainActivity
// Remove current ReaderFragment. Bug Fix #4377
coreMainActivity.navController.popBackStack(
coreMainActivity.readerFragmentRoute,
inclusive = true
)
// Launch fresh ReaderFragment so all the previous arguments will remove.
coreMainActivity.navController.navigate(coreMainActivity.readerFragmentRoute)
// Pass search result to the *new* instance
activity.setNavigationResultOnCurrent(searchString, FIND_IN_PAGE_SEARCH_STRING)
activity.setNavigationResult(searchString, FIND_IN_PAGE_SEARCH_STRING)
activity.popNavigationBackstack()
}
companion object {

View File

@ -19,13 +19,12 @@
package org.kiwix.kiwixmobile.core.search.viewmodel.effects
import android.content.Intent
import androidx.navigation.NavHostController
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkConstructor
import io.mockk.verifyOrder
import io.mockk.verify
import org.junit.jupiter.api.Test
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResultOnCurrent
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.popNavigationBackstack
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResult
import org.kiwix.kiwixmobile.core.main.CoreMainActivity
import org.kiwix.kiwixmobile.core.main.FIND_IN_PAGE_SEARCH_STRING
@ -34,20 +33,11 @@ internal class SearchInPreviousScreenTest {
fun `invoke with returns positive result with string to previous screen`() {
val searchString = "search"
mockkConstructor(Intent::class)
// Mock the activity & navController
val mockNavController = mockk<NavHostController>(relaxed = true)
val activity = mockk<CoreMainActivity>(relaxed = true) {
every { readerFragmentRoute } returns "readerRoute"
every { navController } returns mockNavController
}
val activity = mockk<CoreMainActivity>(relaxed = true)
SearchInPreviousScreen(searchString).invokeWith(activity)
verifyOrder {
mockNavController.popBackStack("readerRoute", true)
mockNavController.navigate("readerRoute")
activity.setNavigationResultOnCurrent(searchString, FIND_IN_PAGE_SEARCH_STRING)
verify {
activity.setNavigationResult(searchString, FIND_IN_PAGE_SEARCH_STRING)
activity.popNavigationBackstack()
}
}
}

View File

@ -179,6 +179,8 @@ class CustomMainActivity : CoreMainActivity() {
}
override fun openSearch(searchString: String, isOpenedFromTabView: Boolean, isVoice: Boolean) {
// remove the previous backStack entry with old arguments.
removeArgumentsOfReaderScreen()
navigate(
CustomDestination.Search.createRoute(
searchString = searchString,
@ -221,6 +223,25 @@ class CustomMainActivity : CoreMainActivity() {
// Do nothing since custom apps does not have the bottomAppBar.
}
/**
* Handles navigation from the left drawer to the Reader screen.
*
* Clears any existing Reader back stack entry (with its arguments)
* and replaces it with a fresh Reader screen using default arguments.
* This ensures old arguments are not retained when navigating
* via the left drawer.
*/
override fun removeArgumentsOfReaderScreen() {
if (navController.currentDestination?.route?.startsWith(readerFragmentRoute) == true) {
navigate(
readerFragmentRoute,
NavOptions.Builder()
.setPopUpTo(CustomDestination.Reader.route, inclusive = true)
.build()
)
}
}
// Outdated shortcut id(new_tab)
// Remove if the application has the outdated shortcut.
private fun removeOutdatedIdShortcuts() {