Skip to content

Untangle http:// and wordpress:// deeplinking URLs#11550

Merged
hypest merged 3 commits intodevelopfrom
issue/untangle-deeplinking-urls
Mar 30, 2020
Merged

Untangle http:// and wordpress:// deeplinking URLs#11550
hypest merged 3 commits intodevelopfrom
issue/untangle-deeplinking-urls

Conversation

@maxme
Copy link
Copy Markdown
Contributor

@maxme maxme commented Mar 27, 2020

Fixes an issue that made wordpress:// URLs non-functional.

This was probably introduced in: #9957

Note that I'm not sure why the issue is happening, according to the documentation this should work fine, and other urls like: http://viewpost should also work as well (which is an unwanted side effect).

Test

Before the patch, run:

adb shell am start \
 -W -a android.intent.action.VIEW \
 -c android.intent.category.BROWSABLE \
 -d "wordpress://viewpost?blogId=124661775\&postId=1194"

The app won't open.

After the patch, run the same command:

adb shell am start \
 -W -a android.intent.action.VIEW \
 -c android.intent.category.BROWSABLE \
 -d "wordpress://viewpost?blogId=124661775\&postId=1194"

The app will open and show the specified post in the reader.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@maxme maxme added the Core label Mar 27, 2020
@maxme maxme added this to the 14.6 milestone Mar 27, 2020
@maxme maxme requested review from hypest and malinajirka March 27, 2020 17:27
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Mar 27, 2020 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@hypest
Copy link
Copy Markdown
Contributor

hypest commented Mar 30, 2020 •
edited
Loading

Test results:

Testing the wordpress custom scheme

✅ wordpress://viewpost?blogId=124661775\&postId=1194
✅ wordpress://stats
✅ wordpress://notifications
✅ wordpress://read
✅ wordpress://post

Testing a normal link to a post

✅ Opens in the app in read-specific-post mode https://stefanosuser.wordpress.com/2020/02/20/comments-issue/

Tests from #9957

✅ Should not open the app: https://wordpress.com/post/
✅ Should not open the app: https://public-api.wordpress.com/nope/
✅ Should not open the app: https://public-api.wordpress.com/bar/?redirect_to=https%3A%2F%2FCHANGEME.wordpress.com%2Fpost
✅ Should open the app's editor on previously selected site: https://wordpress.com/post/wrong-hostname
✅ Should open the app's editor with selected site: https://wordpress.com/post/stefanosuser.wordpress.com
✅ Should open the app's editor with selected site: https://public-api.wordpress.com/mbar/?redirect_to=https%3A%2F%2Fwordpress.com%2Fpost%2Fstefanosuser.wordpress.com
✅ Should "open" the app, but then redirect to the browser: https://public-api.wordpress.com/mbar/?redirect_to=https%3A%2F%2Fwordpress.com%2Fnope

hypest
hypest approved these changes Mar 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix @maxme ! LGTM!

@hypest
Copy link
Copy Markdown
Contributor

hypest commented Mar 30, 2020 •
edited
Loading

Feel free to merge @maxme after updating from develop to resolve the merge conflict 👍

Edit: merged it after seeing the update happened.

@hypest hypest merged commit 63c6399 into develop Mar 30, 2020
@hypest hypest deleted the issue/untangle-deeplinking-urls branch March 30, 2020 13:37
@adityabhaskar
Copy link
Copy Markdown
Contributor

adityabhaskar commented May 11, 2020

@maxme Tangentially related: Is it possible to send an intent to open a post for editing in the WordPress app? Something on the lines of:
wordpress://post?blogId=[site_id]&postId=[post_id] or https://wordpress.com/post/[blog_id]/[post_id].

At the moment they both just open the post editor for a blank new post in the currently selected site.

The reference code starts here:

WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/DeepLinkingIntentReceiverActivity.java

Line 133 in 31cf4fe

private void handleOpenEditor(@NonNull Uri uri) {

@maxme
Copy link
Copy Markdown
Contributor Author

maxme commented May 12, 2020

@adityabhaskar That's a good idea and this would be possible if we changed that code a bit to capture the blog id and then open the editor for this specific site (similar to what is done with the "Share with" feature).

@adityabhaskar
Copy link
Copy Markdown
Contributor

adityabhaskar commented May 13, 2020

@maxme Should I create a new issue with this and tag you in it?

@maxme
Copy link
Copy Markdown
Contributor Author

maxme commented May 13, 2020

Yes please!

@adityabhaskar adityabhaskar mentioned this pull request May 13, 2020
Closed
@renanferrari renanferrari mentioned this pull request Jul 8, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka Awaiting requested review from malinajirka

1 more reviewer

@hypest hypest hypest approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Core

Projects

None yet

Milestone

14.6

Development

Successfully merging this pull request may close these issues.

3 participants

@maxme @hypest @adityabhaskar