Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions WordPress/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,18 @@
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" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class ReaderLinkHandler
* `wordpress://viewpost?blogId={blogId}&postId={postId}`
* wordpress.com/read/feeds/feedId/posts/feedItemId
* wordpress.com/read/blogs/feedId/posts/feedItemId
* wordpress.com/reader/feeds/feedId/posts/feedItemId
* domain.wordpress.com/2.../../../postId
* domain.wordpress.com/19../../../postId
*/
Expand Down Expand Up @@ -95,9 +96,11 @@ class ReaderLinkHandler
buildString {
val segments = uri.pathSegments
// Handled URLs look like this: http[s]://wordpress.com/read/feeds/{feedId}/posts/{feedItemId}
// with the first segment being 'read'.
// or http[s]://wordpress.com/reader/feeds/{feedId}/posts/{feedItemId}
// with the first segment being 'read' or 'reader'.
append(stripHost(uri))
if (segments.firstOrNull() == "read") {
val firstSegment = segments.firstOrNull()
if (firstSegment == "read" || firstSegment == "reader") {
appendReadPath(segments)
} else if (segments.size > DATE_URL_SEGMENTS) {
append("/YYYY/MM/DD/$POST_ID")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,12 @@ class ReaderPostPagerActivity : BaseAppCompatActivity() {
val segments = uri.pathSegments

// Handled URLs look like this: http[s]://wordpress.com/read/feeds/{feedId}/posts/{feedItemId}
// with the first segment being 'read'.
// or http[s]://wordpress.com/reader/feeds/{feedId}/posts/{feedItemId}
// with the first segment being 'read' or 'reader'.
if (segments != null) {
// Builds stripped URI for tracking purposes
val wrappedUri = UriWrapper(uri)
if (segments[0] == "read") {
if (segments[0] == "read" || segments[0] == "reader") {
if (segments.size > 2) {
blogIdentifier = segments[2]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,16 @@ class ReaderLinkHandlerTest : BaseUnitTest() {
assertThat(strippedUrl).isEqualTo("wordpress.com/read/blogs/feedId/posts/feedItemId")
}

@Test
fun `correctly strips reader feeds URI`() {
val uri = buildUri("wordpress.com", "reader", "feeds", feedId.toString(), "posts", postId.toString())

val strippedUrl = readerLinkHandler.stripUrl(uri)

// Note: both 'reader' and 'read' paths are normalized to 'read' for analytics tracking
assertThat(strippedUrl).isEqualTo("wordpress.com/read/feeds/feedId/posts/feedItemId")
}

@Test
fun `correctly strips 2xxx URI`() {
val uri = buildUri("wordpress.com", "2020", "10", "1", postId.toString())
Expand Down
22 changes: 22 additions & 0 deletions config/lint/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4918,6 +4918,28 @@
column="17"/>
</issue>

<issue
id="IntentFilterUniqueDataAttributes"
message="Consider splitting data tag into multiple tags with individual attributes to avoid confusion"
errorLine1=" &lt;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=" &lt;data"
errorLine2=" ^">
<location
file="src/main/AndroidManifest.xml"
line="666"
column="17"/>
</issue>

Comment on lines +4921 to +4942
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 updateLintBaseline

Option 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thank you for the explanation.

<issue
id="NonConstantResourceId"
message="Resource IDs will be non-final by default in Android Gradle Plugin version 8.0, avoid using them in switch case statements"
Expand Down