Skip to content

Commit aedfc20

Browse files
author
Jeremy Woods
committedApr 5, 2023
Ensure manually adding a fragment to the back stack fails
If you are using Navigation and do a manual fragment transaction that attempts to add a fragment to the fragment manager's back stack, the fragment manager and Navigation back stacks will fall out of sync and cause unexpected behavior. Instead of allowing this, we should always throw whenever a fragment transaction attempts to add a fragment to the fragment manager back stack. RelNote: "When using Navigation with Fragments, attempting to manually do a `FragmentTransaction` that adds a fragment to the `FragmentManager`'s back stack will now throw an `IllegalArgumentException`. You should always add fragments via the `navigate()` API." Test: added FragmentNavigatorTest Change-Id: I6d38e3fac4b7552881655d18304d5efec6168d03
1 parent 968bf87 commit aedfc20

File tree

2 files changed

+45
-1
lines changed

2 files changed

+45
-1
lines changed
 

‎navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt

+38-1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import androidx.testutils.withActivity
5353
import androidx.testutils.withUse
5454
import com.google.common.truth.Truth.assertThat
5555
import com.google.common.truth.Truth.assertWithMessage
56+
import java.lang.IllegalArgumentException
5657
import java.util.concurrent.CountDownLatch
5758
import java.util.concurrent.TimeUnit
5859
import kotlin.reflect.KClass
@@ -555,7 +556,7 @@ class FragmentNavigatorTest {
555556

556557
@UiThreadTest
557558
@Test
558-
fun testNavigateAndAddIndependentFragment() {
559+
fun testNavigationAndAddIndependentFragmentWithoutBackStack() {
559560
val entry = createBackStackEntry()
560561

561562
fragmentNavigator.navigate(listOf(entry), null, null)
@@ -584,6 +585,42 @@ class FragmentNavigatorTest {
584585
.isEqualTo(independentFragment)
585586
}
586587

588+
@UiThreadTest
589+
@Test
590+
fun testNavigateAndAddIndependentFragmentWithBackStack() {
591+
val entry = createBackStackEntry()
592+
593+
fragmentNavigator.navigate(listOf(entry), null, null)
594+
assertThat(navigatorState.backStack.value)
595+
.containsExactly(entry)
596+
fragmentManager.executePendingTransactions()
597+
val fragment = fragmentManager.findFragmentById(R.id.container)
598+
assertWithMessage("Fragment should be added")
599+
.that(fragment)
600+
.isNotNull()
601+
assertWithMessage("Fragment should be the correct type")
602+
.that(fragment)
603+
.isInstanceOf(EmptyFragment::class.java)
604+
assertWithMessage("Fragment should be the primary navigation Fragment")
605+
.that(fragmentManager.primaryNavigationFragment)
606+
.isSameInstanceAs(fragment)
607+
608+
val independentFragment = EmptyFragment()
609+
fragmentManager.beginTransaction()
610+
.replace(R.id.container, independentFragment)
611+
.addToBackStack(null)
612+
.commit()
613+
try {
614+
fragmentManager.executePendingTransactions()
615+
} catch (e: IllegalArgumentException) {
616+
assertWithMessage("adding a fragment to the back stack manually should fail")
617+
.that(e.message)
618+
.contains("The fragment " + independentFragment + " is unknown to the " +
619+
"FragmentNavigator. Please use the navigate() function to add fragments to " +
620+
"the FragmentNavigator managed FragmentManager.")
621+
}
622+
}
623+
587624
@LargeTest
588625
@UiThreadTest
589626
@Test

‎navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt

+7
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ public open class FragmentNavigator(
143143
val entry = (state.backStack.value + state.transitionsInProgress.value).lastOrNull {
144144
it.id == fragment.tag
145145
}
146+
if (!pop) {
147+
requireNotNull(entry) {
148+
"The fragment " + fragment + " is unknown to the FragmentNavigator. " +
149+
"Please use the navigate() function to add fragments to the " +
150+
"FragmentNavigator managed FragmentManager."
151+
}
152+
}
146153
if (entry != null) {
147154
// In case we get a fragment that was never attached to the fragment manager,
148155
// we need to make sure we still return the entries to their proper final state.

0 commit comments

Comments
 (0)