Support new "reader" deeplinks #22369
Conversation
Generated by 🚫 Danger |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/base_manifest.txt 2025-11-26 14:32:11.601652526 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/head_manifest.txt 2025-11-26 14:32:13.641648442 +0000
@@ -682,6 +682,16 @@
android:pathPattern="/19../../../.*"
android:scheme="http" >
</data>
+ <data
+ android:host="wordpress.com"
+ android:pathPattern="/reader/feeds/.*/posts/.*"
+ android:scheme="https" >
+ </data>
+ <data
+ android:host="wordpress.com"
+ android:pathPattern="/reader/feeds/.*/posts/.*"
+ android:scheme="http" >
+ </data>
<action android:name="org.wordpress.android.action.VIEW_POST" />
<action android:name="android.intent.action.VIEW" />Go to https://buildkite.com/automattic/wordpress-android/builds/23994/canvas?sid=019ac090-2ae0-484a-9261-4b747d249d9f, click on the |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/base_manifest.txt 2025-11-26 14:33:25.181295999 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/head_manifest.txt 2025-11-26 14:33:28.661306592 +0000
@@ -709,6 +709,16 @@
android:pathPattern="/19../../../.*"
android:scheme="http" >
</data>
+ <data
+ android:host="wordpress.com"
+ android:pathPattern="/reader/feeds/.*/posts/.*"
+ android:scheme="https" >
+ </data>
+ <data
+ android:host="wordpress.com"
+ android:pathPattern="/reader/feeds/.*/posts/.*"
+ android:scheme="http" >
+ </data>
<action android:name="org.wordpress.android.action.VIEW_POST" />
<action android:name="android.intent.action.VIEW" />Go to https://buildkite.com/automattic/wordpress-android/builds/23994/canvas?sid=019ac090-2ae1-4a55-aa9f-bb3d6b68054a, click on the |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22369-0d96c5b | |
| Commit | 0d96c5b | |
| Direct Download | wordpress-prototype-build-pr22369-0d96c5b.apk |
|
| App Name | Jetpack | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22369-0d96c5b | |
| Commit | 0d96c5b | |
| Direct Download | jetpack-prototype-build-pr22369-0d96c5b.apk |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
| <issue | ||
| id="IntentFilterUniqueDataAttributes" | ||
| message="Consider splitting data tag into multiple tags with individual attributes to avoid confusion" | ||
| errorLine1=" <data" | ||
| errorLine2=" ^"> | ||
| <location | ||
| file="src/main/AndroidManifest.xml" | ||
| line="660" | ||
| column="17"/> | ||
| </issue> | ||
|
|
||
| <issue | ||
| id="IntentFilterUniqueDataAttributes" | ||
| message="Consider splitting data tag into multiple tags with individual attributes to avoid confusion" | ||
| errorLine1=" <data" | ||
| errorLine2=" ^"> | ||
| <location | ||
| file="src/main/AndroidManifest.xml" | ||
| line="666" | ||
| column="17"/> | ||
| </issue> | ||
|
|
There was a problem hiding this comment.
The new deeplink addition is adding a couple of lint issues. I explored some solutions, but ended up realising that they could potentially add non-expected cases.
I followed the already in-place deeplink matching, and it's actually adding the lint check suppression. So, the most realistic solution was simply following on what we are already doing.
In fact, according to Claude:
Option 1: Update the baseline (quickest fix, consistent with existing code)
./gradlew updateLintBaselineOption 2: Split the data tags (proper fix following Android lint recommendations)
The data tags should be split like this:
<data android:scheme="https" />
<data android:host="wordpress.com" />
<data android:pathPattern="/reader/feeds/./posts/." />However, this changes behavior because intent filter matching combines ALL data tags in an OR fashion for each attribute type. This could cause unintended matches.
Given that the existing codebase has 700+ similar issues already baselined, the most consistent approach is to update the baseline
So, I went with solution 1
There was a problem hiding this comment.
Makes sense. Thank you for the explanation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #22369 +/- ##
=======================================
Coverage 39.02% 39.03%
=======================================
Files 2203 2203
Lines 106327 106328 +1
Branches 15059 15059
=======================================
+ Hits 41499 41500 +1
Misses 61338 61338
Partials 3490 3490 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dcalhoun
left a comment
dcalhoun
left a comment
There was a problem hiding this comment.
This looks good. The test plan succeeded for me.
I will note, I tested a slight variation where I arrived at the Calypso app promotion banner. I noted that tapping the Open in the Jetpack app loads Reader, but not the individual post detail view. This a bit unexpected from my perspective. It'd be ideal if we could open the current post viewed in the browser.
The relevant web code is found in wp-calypso. If I'm not mistaken, we do not appear to pass along any details regarding the post. If we did, could the Android app consume these details and open the post? What details would be needed? What properties should be set?
Screen_Recording_20251126_101239_Chrome.mp4
| <issue | ||
| id="IntentFilterUniqueDataAttributes" | ||
| message="Consider splitting data tag into multiple tags with individual attributes to avoid confusion" | ||
| errorLine1=" <data" | ||
| errorLine2=" ^"> | ||
| <location | ||
| file="src/main/AndroidManifest.xml" | ||
| line="660" | ||
| column="17"/> | ||
| </issue> | ||
|
|
||
| <issue | ||
| id="IntentFilterUniqueDataAttributes" | ||
| message="Consider splitting data tag into multiple tags with individual attributes to avoid confusion" | ||
| errorLine1=" <data" | ||
| errorLine2=" ^"> | ||
| <location | ||
| file="src/main/AndroidManifest.xml" | ||
| line="666" | ||
| column="17"/> | ||
| </issue> | ||
|
|
There was a problem hiding this comment.
Makes sense. Thank you for the explanation.
Good catch! Let me check it. |
After some investigation, you are right, we are not passing any post details. Notice the Basically, we would need to add post info like in the original URL, assuming it0s the blog and post ids |
Description
This PR adds support for the "reader" deeplinks
Testing instructions
https://wordpress.com/read/feeds/138734090/posts/5879194632
https://wordpress.com/reader/feeds/138734090/posts/5879194632