Rename screenName to deeplinkScreenName and widen deeplink coverage - #9577
Open
catalinradoiu wants to merge 1 commit into
Open
Rename screenName to deeplinkScreenName and widen deeplink coverage#9577catalinradoiu wants to merge 1 commit into
catalinradoiu wants to merge 1 commit into
Conversation
Rename the @ContributeToActivityStarter parameter to deeplinkScreenName so it reads as what it is, and declare a name for the screens that make sense as a deeplink entry point. Screens deliberately left without a name: mid-flow screens, screens needing caller context, WebView hosts that load a caller-provided URL, and everything in internal/dev modules. Sync had no deeplink at all because its mapper is handwritten to switch between SyncActivity and SyncActivityV2; it now resolves "sync". The generated mapper only built params from an object instance or a non-empty payload, so a params type whose values all default (subscriptions.restore) could not resolve. It now falls back to parsing an empty payload. Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1213980692214675 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1213980692214675
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable): None
Description
Two things, per the task.
1.
screenName->deeplinkScreenNameon@ContributeToActivityStarter. The old name read like "the name of the screen" rather than "the name this screen is deeplinked by". Only the KSP processor reads the argument, so this is the annotation, the processor, the golden files, and the 27 existing call sites. The KDoc now also states the naming convention, the uniqueness requirement, and which screens should not declare a name.DeeplinkActivityParams.screenNameis deliberately left alone: it already sits inside aDeeplink*type, sodeeplinkActivityParams.deeplinkScreenNamestutters, and renaming it would be an-apichange for no clarity gain.2. Declared a name for 25 screens that are sensible deeplink entry points, so they can be targeted from a What's New message. Names follow the existing
<feature>.<subScreen>convention, each segment camelCase:about,privateSearch,webTrackingProtection,threatProtection,dataClearing,permissions,sitePermissions,downloads,duckplayer.settingsmacos,windows,syncvpn.geoswitching,vpn.exclusions,vpn.customDns,apptp.onboardingpasswords,passwords.import,autofill.settingssubscriptions.restore,subscriptions.changePlan,ppro.feedback,pir,pir.dashboard,duckai.paidSettingsScreens deliberately left without a name:
WebViewActivity,SettingsWebViewActivity,SubscriptionsWebViewActivityWithParams,PdfViewerActivity. A deeplink name there would let an RMF payload choose the URL loaded inside the browser's own trusted chrome. (The fixed-destinationsubscriptions.purchase/subscriptions.upgradevariants are unaffected.)AutofillPasswordsManagementViewCredential,PrivacyDashboardHybridActivity(needs a tab), the breakage reporting flow,ModalSurfaceActivity, and others.*-internalmodules and internal build variants (15 activities).Two things fell out of the audit and are fixed here:
SyncActivityParamMapperis handwritten (it switches betweenSyncActivityandSyncActivityV2behinduseSimplifiedSync) and returnednullfor every deeplink. It now resolves"sync", with the flag switching intact.objectInstanceor from a non-empty payload, sosubscriptions.restore(adata classwith all-default values) silently returnednullon an empty payload. Mappers now fall back to parsing"{}".Note for whoever writes the RMF message:
passwordsandautofill.settingsneed{"source":"Unknown"}as the payload — theirsourcehas no default. Everything else works with no payload.Steps to test this PR
Deeplinks resolve
cards_listmessage on themodalsurface withnavigationactions for the names in the table above -> "Download config now".id), reopen, and confirm the What's New modal appears with every entry.Payload-carrying screens
passwordsandautofill.settingsopen with{"source":"Unknown"}.subscriptions.restoreopens with an empty payload (this is the codegen fallback above; it returned null before this PR).Sync
navigationaction with valuesyncopens the sync screen, withuseSimplifiedSyncboth on and off.Nothing regressed
settings,tabSwitcherWithParamswith{"browserMode":"FIRE"}.UI changes
Note
Medium Risk
Widens remote-message navigation into more in-app screens and changes generated deeplink param construction. Risk is mainly mis-routed RMF actions or silent message drops from name/payload mismatches, not auth or data handling.
Overview
Renames
@ContributeToActivityStarter'sscreenNametodeeplinkScreenNameso the annotation reads as a deeplink identifier, not a screen title. Docs and KSP now spell out uniqueness,<feature>.<subScreen>naming, and which screens must not opt in (mid-flow, caller context, caller-provided URLs, internal modules).DeeplinkActivityParams.screenNameis unchanged.Many landing/settings screens now declare a name so RMF can open them. Existing names are migrated in place. WebView hosts that take a URL, mid-flow screens, and internal variants stay unnamed.
Generated mappers fall back to Moshi
"{}"when there is no payload, so data classes with all-default fields (e.g. restore subscription) resolve instead of returning null. HandwrittenSyncActivityParamMappernow maps"sync"toSyncActivityWithEmptyParamswhile still switching V1/V2 viauseSimplifiedSync.Reviewed by Cursor Bugbot for commit 3f04b7a. Bugbot is set up for automated code reviews on this repo. Configure here.