Deeplink matches with wrong url when sharing deeplink action [395712033]

Fixed

Bug

Status Update

No update yet.

Description

cl...@google.com

created issue #1

Feb 10, 2025 11:14PM

If a NavGraph has multiple NavDestinations, each with a deeplink that share the same action i.e.

composable(
    route = "A",
    deepLinks = listOf(
        navDeepLink {
            uriPattern = "https://example.com/{id}"
            action = Intent.ACTION_VIEW
        },
    ),
    ...
)

composable(
    route = "B",
    deepLinks = listOf(
        navDeepLink {
            uriPattern = "https://completelyDifferentUrl.com"
            action = Intent.ACTION_VIEW
        },
    ),
    ...
)

Deeplink "https://example.com/2" would actually match with https://completelyDifferentUrl.com rather than the exact match https://example.com/{id}.

We would expect deeplink to match with the url that has a better path + argument match.

Comments

lo...@gmail.com <lo...@gmail.com> #2Feb 12, 2025 10:11AM

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

Message last modified on Feb 12, 2025 10:12AM

cl...@google.com <cl...@google.com> #3Feb 12, 2025 05:55PM

It's essentially the same cause - navigation is prioritizing the wrong deeplink when they share the same action.

cl...@google.com <cl...@google.com> Feb 19, 2025 05:37PM

Accepted by cl...@google.com.

cl...@google.com <cl...@google.com> #4Feb 19, 2025 10:50PM

Marked as fixed.

Fixed internally and will be available in navigation-2.8.8

ap...@google.com <ap...@google.com> #5Feb 19, 2025 10:50PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Clara Fok <clarafok@google.com>
Link: https://android-review.googlesource.com/3490354

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

ba...@gocity.com <ba...@gocity.com> #6Feb 27, 2025 12:37PM

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

cl...@google.com <cl...@google.com> #7Feb 27, 2025 07:24PM

comment#6 I replicated your setup and the deeplink worked for me.

@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?

Message last modified on Feb 27, 2025 07:24PM

e....@gmail.com <e....@gmail.com> #8Mar 4, 2025 10:29AM

comment#6, comment#7

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

ba...@gocity.com <ba...@gocity.com> #9Mar 11, 2025 09:08AM

comment#7,comment#8 I created a bug for this with a simple project to reproduce it, and it was already a duplicate of another known bug in 2.8.8.

cl...@google.com <cl...@google.com> #10Mar 11, 2025 03:52PM

The bug fix for comment#6, comment#8 will be in 2.8.9.