Supplying an empty list for a startDestination's argument causes app to crash [341723133]

Fixed

Bug

Status Update

No update yet.

Description

do...@google.com

created issue #1

May 20, 2024 05:03PM

Component used: Navigation

Version used: 2.8.0-beta01

Devices/Android versions reproduced on: all

When specifying the startDestination for a NavHost, if the destination class is instantiated with an empty list for one of its properties, the app will crash.

Repro code:

@Composable
fun Content() {
   val navController = rememberNavController()
   NavHost(navController = navController, startDestination = Home(usernames = emptyList())) {
       composable<Home> { backStackEntry ->
           val home: Home = backStackEntry.toRoute()
           for (username in home.usernames) {
               Text(username)
           }
       }
   }
}

@Serializable
data class Home(
   val usernames: List<String>,
)

Expected outcome:

App runs and displays an empty screen

Actual outcome:

App crashes with the following error in logcat:

java.lang.IllegalArgumentException: navigation destination com.example.hellonavigation.Home is not a direct child of this NavGraph

Possibly related to: https://b.corp.google.com/issues/339481310

Comments

jb...@google.com <jb...@google.com> May 21, 2024 02:01PM

Assigned to cl...@google.com.

cl...@google.com <cl...@google.com> #2May 21, 2024 08:29PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3Jun 5, 2024 12:36AM

Project: platform/frameworks/support
Branch: androidx-main

commit 0f731acf7753ca32ba38c44c5f325549fb920de1
Author: Clara Fok <clarafok@google.com>
Date: Mon Jun 03 14:14:38 2024

Add emptyCollection API for CollectionNavType

To support empty collections (i.e. emptyList()) when navigating with args, we need a way to create an empty collection of type T.

Test: existing tests
Bug: 341723133
Relnote: "Added new abstract emptyCollection() method for CollectionNavType to support navigating with a empty collection of type T."
Change-Id: Ie4d8464050a7164176c2aee55518b03a5db704ad

M navigation/navigation-common/api/2.8.0-beta03.txt
M navigation/navigation-common/api/current.txt
M navigation/navigation-common/api/restricted_2.8.0-beta03.txt
M navigation/navigation-common/api/restricted_current.txt
M navigation/navigation-common/src/androidTest/java/androidx/navigation/serialization/RouteFilledTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/CollectionNavType.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavType.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/NavArgumentGeneratorTest.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/RoutePatternTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt
M navigation/navigation-testing/src/androidTest/java/androidx/navigation/testing/TestSavedStateHandleFactory.kt

https://android-review.googlesource.com/3112740

ap...@google.com <ap...@google.com> #4Jun 5, 2024 02:11AM

Project: platform/frameworks/support
Branch: androidx-main

commit cc3786dcd8fb1dc3db40d2dbcf878003a6eb5fdb
Author: Clara Fok <clarafok@google.com>
Date: Fri May 31 17:55:16 2024

Refactor NavDeepLink.parseInputParams

Refactored so that it is more readable. It should now be more obvious that when parsing repeated params, we actually call parseArgument first, and call parseArgumentForRepeatedParam for subsequent arg values.

This refactor will also make it easier to handle empty collections for upcoming changes.

Test: ./gradlew navigation:navigation-compose:cC
Bug: 341723133

Change-Id: Ib790ef41a41709842afc01da783ede8a70e251d8

M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.kt
M navigation/navigation-compose/src/androidTest/java/androidx/navigation/compose/NavHostControllerTest.kt

https://android-review.googlesource.com/3112741

ap...@google.com <ap...@google.com> #5Jun 6, 2024 06:19PM

Project: platform/frameworks/support
Branch: androidx-main

commit fb14567d675ea2f166937f5639430bb2aa0d3a71
Author: Clara Fok <clarafok@google.com>
Date: Mon Jun 03 16:08:07 2024

Handle empty CollectionNavType

For collection types, in the past if no value was passed in, the arg would be ignored and omitted from argument bundle. Now, a missing collection type value will default to the NavArgument's defaultValue, or an emptyList if there isn't a defaultValue.

What is considered "no value" is different between list of strings and list of non-strings. For clarity, this is how various routes are handled:

Given a route pattern "

www.test.com?list={myList}"

List<String>
1. "

www.test.com?list=" considered as listOf("")
2. "

www.test.com?list={myList}" considered as listOf("{myList}")
3. "

www.test.com?" considered as no value
4. "

www.test.com" considered as no value

non-String lists
1. "

www.test.com?list=" considered as no value (since it is invalid)
2. "

www.test.com?list={myList}" considered as no value
3. "

www.test.com?" considered as no value
4. "

www.test.com" considered as no value

Test: ./gradlew navigation:navigation-common:cC
Test: ./gradlew navigation:navigation-runtime:cC
Bug: 341723133
Relnote: "Fix how navigation handles missing arg values for CollectionNavTypes - instead of ignoring missing values, navigation will now default to its defaultValue, or an empty collection if there isn't a defaultValue. This fallback value will be added the NavBackStackEntry.argument bundle."

Change-Id: Id630fb148782fbb34d3c236f5d8bb363fba7ef91

M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/androidTest/java/androidx/navigation/test/NavArgument.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

https://android-review.googlesource.com/3112742

ap...@google.com <ap...@google.com> #6Jun 6, 2024 06:19PM

Project: platform/frameworks/support
Branch: androidx-main

commit 1ae187912afa613f6c1dad408017ebc4b0c3cc41
Author: Clara Fok <clarafok@google.com>
Date: Mon Jun 03 16:52:11 2024

Add safe args tests for empty lists

Test: ./gradlew navigation:navigation-common:cC
Test ./gradlew navigation:navigation-runtime:cC
Bug: 341723133

Change-Id: I1c67ff34424a4559ed09ad2ef1f92c7baec2e23a

M navigation/navigation-common/src/androidTest/java/androidx/navigation/serialization/RouteFilledTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

https://android-review.googlesource.com/3112743

cl...@google.com <cl...@google.com> #7Jun 6, 2024 06:41PM

Marked as fixed.

Fixed internally and will be available in navigation 2.8.0-beta03.

pr...@google.com <pr...@google.com> #8Jun 12, 2024 10:45PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.navigation:navigation-common:2.8.0-beta03
androidx.navigation:navigation-compose:2.8.0-beta03
androidx.navigation:navigation-runtime:2.8.0-beta03