Status Update
Comments
ku...@google.com <ku...@google.com> #2
this issue was created using original issue https://issuetracker.google.com/issues/389970341#comment2
.
but i am not sure that described text above is the same as in original issue:
also doesn't work
https://example.com/path/detail?token=123 in all versions.
pattern:
https://example.com/path/.*?token={id}
P.S. original issue also has test project https://issuetracker.google.com/389970341#attachment62385890
ku...@google.com <ku...@google.com> #3
It's essentially the same cause - navigation is prioritizing the wrong deeplink when they share the same action.
ku...@google.com <ku...@google.com> #4
Fixed internally and will be available in navigation-2.8.8
ko...@jetbrains.com <ko...@jetbrains.com> #5
Project: platform/frameworks/support
Branch: androidx-main
Author: Clara Fok <
Link:
Fix deeplink incorrectly matching when deep link fields don’t match perfectly
Expand for full commit details
Fix deeplink incorrectly matching when deep link fields don’t match perfectly
Navigation used to allow matching as long as one of three (uri / action / mimType) matches. However this is erroneous, as each field is a defined filter - a potential match should at the very least fulfill all filters.
This is fixed by applying these ground rules for deeplink matching:
1. The added NavDeepLink field’s nullability has to match the NavDeepLinkRequest field’s nullability. i.e. if deeplink.uri is null, request.uri must also be null
2. Any NavDeepLink non-null fields has to match exactly with NavDeepLinkRequest’s non-null fields
To elaborate, consider this scenario where
request:
“www.example.com/13”, action = “theAction”
potential matches:
1. www.example.com/{id}, action = "theAction"
2. www.differentUrl.com, action = "theAction"
When handling this request , even though deeplink 2 has a matching action, the differing url will rule it out as a potential match.
This fix also addresses a bug with wildcards where
request:
www.example.com/wildCardMatch?token=123, action = "theAction"
potential matches:
1. www.example.com/.*?token={id}, action = "theAction"
2. www.example.com?token={id}, action = "theAction"
Even though both deep links have a matching Action, deeplink 1 has a better Uri match, and so the request will now match with deeplink 1 instead of deeplink 2.
Test: ./gradlew navigation:navigation-common:cC
Test: ./gradlew navigation:navigation-runtime:cC
Bug: 395712033
Relnote: “NavDeepLink matching has been fixed where a deeplink and a deeplink request have to match exactly on uri, action, and mime. Matching is no longer allowed if only one or two fields match.“
Change-Id: I3b0295caa6324cc707d080856e88e62b4c3cd4d5
Files:
- M
navigation/navigation-common/src/androidInstrumentedTest/kotlin/androidx/navigation/NavGraphAndroidTest.kt
- M
navigation/navigation-common/src/androidMain/kotlin/androidx/navigation/NavDestination.android.kt
- M
navigation/navigation-common/src/commonMain/kotlin/androidx/navigation/NavDeepLink.kt
- M
navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/BaseNavControllerTest.kt
- M
navigation/navigation-runtime/src/androidInstrumentedTest/kotlin/androidx/navigation/NavControllerRouteTest.kt
- M
navigation/navigation-runtime/src/androidInstrumentedTest/kotlin/androidx/navigation/NavControllerTest.kt
- M
navigation/navigation-runtime/src/androidInstrumentedTest/kotlin/androidx/navigation/NavInflaterTest.kt
- M
navigation/navigation-runtime/src/androidInstrumentedTest/res/navigation/nav_deeplink.xml
- M
navigation/navigation-runtime/src/androidInstrumentedTest/res/navigation/nav_simple.xml
Hash: 85ccd3f102d5d1da4bfc7ceba71425319a6f1c50
Date: Wed Feb 19 12:51:38 2025
mg...@google.com <mg...@google.com> #6
I think this fix has broken some other deeplinks.
I have the following defined in my NavHost file:
composable<Screen.AttractionDetail>(deepLinks = listOf(navDeepLink<Screen.AttractionDetail>(basePath = "gocity:/attraction"))) ...
@Serializable
data class AttractionDetail(val id: String, val hideBookmark: Boolean = false) : Screen
In one of my UI tests I have this code:
deeplinkNavigator("gocity:/attraction/$attractionId")
With 2.8.7 this works fine but as soon as I upgrade to 2.8.8 the deeplink no longer opens the AttractionDetail screen.
I've experimented with changing the data class to be val hideBookmark: Boolean?
and to using uriPattern
instead of basePath
but I can't get this deeplink to work in 2.8.8
ap...@google.com <ap...@google.com> #7
@Serializable
sealed interface Screen {
@Serializable
data class AttractionDetail(val id: String, val hideBookmark: Boolean = false) : Screen
}
controller.graph =
controller.createGraph(startDestination = "start") {
...
composable<Screen.AttractionDetail> { deepLink(navDeepLink<Screen.AttractionDetail>("gocity:/attraction")) }
}
val request = NavDeepLinkRequest(Uri.parse("gocity:/attraction/13"), null, null)
val handled = controller.handleDeepLink(request)
Can you file a separate bug and attach a repro?
ap...@google.com <ap...@google.com> #8
I also had problem after updating to version 2.8.8.
Explicitly setting action
to Intent.ACTION_VIEW
in the Deep Link builder now may be required if your intent filter for deeplink schema has android.intent.action.VIEW
.
At least that's how I solved the issue that I got after using version 2.8.8.
See example:
Manifest:
<intent-filter>
<action android:name="android.intent.action.VIEW" />
<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />
<data
android:scheme="company"
android:host="app"
android:pathPattern="/.*" />
</intent-filter>
In broadcast receiver:
//...
val deepLinkUri = NavDeepLinks.progressDeepLink.uriPattern
val intent = Intent(
Intent.ACTION_VIEW,
Uri.parse(deepLinkUri) //
).apply {
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP
`package` = context.packageName
}
val pendingIntent = PendingIntent.getActivity(
context,
0,
intent,
PendingIntent.FLAG_IMMUTABLE
)
//...
Definition of DeepLink using builder:
val progressDeepLink = navDeepLink {
uriPattern = "$BASE/${Path.PROGRESS}"
action = Intent.ACTION_VIEW
}
Compose Navigation Graph Builder:
navigation<LimitsDirection>(
deepLinks = listOf(NavDeepLinks.limitsDeepLink)
) {
LimitsScreen(viewModel = limitsViewModel)
}
I hope that helps you.
Also, may be this fix requires some update for documentation and/or guides
ap...@google.com <ap...@google.com> #11
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Remove getOrElse
in SavedStateReader
Expand for full commit details
Remove `getOrElse` in `SavedStateReader`
- Because `getOrNull()` is now available, users can achieve the same behavior with `getOrNull() ?: else()`, allowing us to reduce the public API surface and the number of inline methods (as `getOrElse` takes a lambda, it cannot be removed).
RelNote: "Remove `getOrElse` from `SavedStateReader` in favor of `getOrNull() ?: else()`."
Bug: 399820614
Test: SavedStateTest
Change-Id: I87317b0eaa0f6d0fad334cc3f81bfcc8a90e6313
Files:
- M
lifecycle/lifecycle-viewmodel-savedstate/src/commonMain/kotlin/androidx/lifecycle/SavedStateHandleSupport.kt
- M
savedstate/savedstate/api/current.txt
- M
savedstate/savedstate/api/restricted_current.txt
- M
savedstate/savedstate/bcv/native/current.txt
- M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/Recreator.android.kt
- M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt
- M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt
- M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt
- M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateTest.kt
- M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: e04b610835745492aa66aa45be1426d57c1401b6
Date: Mon Mar 03 23:42:23 2025
ap...@google.com <ap...@google.com> #12
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Add non-reified get
collections in SavedStateReader
Expand for full commit details
Add non-reified `get` collections in `SavedStateReader`
- Previously, retrieving a typed collection required a `reified` method, restricting its usage at runtime.
- Introducing a non-reified variant allows clients to call it with a dynamic type.
- This change also enables code encapsulation by reducing inlined code, with the reified method simply delegating to the non-reified one.
RelNote: "Add non-reified methods for `get` collections in `SavedStateReader`."
Bug: 399820614
Test: N/A
Change-Id: I0b641cba421761ee41bb8dc386cbc5db7cb67066
Files:
- M
savedstate/savedstate/api/current.txt
- M
savedstate/savedstate/api/restricted_current.txt
- M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt
- M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt
- M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: 481a2c575f9c2c3a71e99cf61f093d079d69e1bc
Date: Tue Mar 04 10:55:25 2025
ap...@google.com <ap...@google.com> #13
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523155]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com <ap...@google.com> #14
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
- androidx.savedstate.SavedStateWriter
[Gerrit:https://android-review.googlesource.com/3523153]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com <ap...@google.com> #15
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523152]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com <ap...@google.com> #16
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523151]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
mg...@google.com <mg...@google.com>
na...@google.com <na...@google.com> #17
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.lifecycle:lifecycle-viewmodel-savedstate:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-android:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-desktop:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iosarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iossimulatorarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iosx64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxx64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-macosarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-macosx64:2.9.0-alpha12
androidx.navigation:navigation-common:2.9.0-alpha08
androidx.navigation:navigation-common-android:2.9.0-alpha08
androidx.navigation:navigation-common-desktop:2.9.0-alpha08
androidx.navigation:navigation-common-iosarm64:2.9.0-alpha08
androidx.navigation:navigation-common-iossimulatorarm64:2.9.0-alpha08
androidx.navigation:navigation-common-iosx64:2.9.0-alpha08
androidx.navigation:navigation-common-linuxarm64:2.9.0-alpha08
androidx.navigation:navigation-common-linuxx64:2.9.0-alpha08
androidx.navigation:navigation-common-macosarm64:2.9.0-alpha08
androidx.navigation:navigation-common-macosx64:2.9.0-alpha08
androidx.navigation:navigation-runtime:2.9.0-alpha08
androidx.navigation:navigation-runtime-android:2.9.0-alpha08
androidx.navigation:navigation-runtime-desktop:2.9.0-alpha08
androidx.navigation:navigation-runtime-iosarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-iossimulatorarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-iosx64:2.9.0-alpha08
androidx.navigation:navigation-runtime-linuxarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-linuxx64:2.9.0-alpha08
androidx.navigation:navigation-runtime-macosarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-macosx64:2.9.0-alpha08
androidx.savedstate:savedstate:1.3.0-alpha10
androidx.savedstate:savedstate-android:1.3.0-alpha10
androidx.savedstate:savedstate-desktop:1.3.0-alpha10
androidx.savedstate:savedstate-iosx64:1.3.0-alpha10
androidx.savedstate:savedstate-linuxarm64:1.3.0-alpha10
androidx.savedstate:savedstate-linuxx64:1.3.0-alpha10
androidx.savedstate:savedstate-macosarm64:1.3.0-alpha10
androidx.savedstate:savedstate-macosx64:1.3.0-alpha10
Description
JetBrains has asked if we could support a
get*OrNull()
variant for our methods. The currentget*OrElse()
does not allow returning null, and checking isNull beforehand is both unnecessary and inefficient.With AOSP/3500556 change to reduce excessive inlining, we plan to move from inline to non-inline functions, as benchmarks show minimal performance impact. This works well for
get*
functions, butget*OrElse()
would allocate lambdas if we don't use inline, which we don't want to.Here are the options before the beta:
Option 1: Do not support
get*OrNull()
.isNull
andcontains
before aget
runs at 60ns instead of 20ns and (2) wouldn’t be able to remove inline fromget*OrElse()
due to lambda allocations.Option 2a: Support
get*OrNull()
, which also allows migratingget*OrElse()
out of inline functions.get*OrElse()
since users can simply callgetOrNull() ?: else()
.get*OrElse()
, so this would require extra work on their side.getOrNull()
seems useful, andBundle
always boxes primitives anyway, since everything is stored in aMap<K, V>
.Edit 03.03.25:
get*()
, and change to return a@Nullable
value.get
function (Map
). (2) Reduces API surface by half.SavedState
is a specializedMap
, not a general-purpose one, so if anACTION_FOO
expectsEXTRA_FOO_ARG1
andEXTRA_FOO_ARG2
, their absence means execution can't proceed. Throwing an error for a missing key is (arguably) reasonable, whileget*OrNull()
/get*OrElse()
provide an explicit opt-out mechanism for cases where a null or default value is acceptable.