Allow loading android-app:// uris

RESOLVED DUPLICATE of bug 1182140

Status

()

RESOLVED DUPLICATE of bug 1182140
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

(Blocks: 3 bugs, {relnote})

unspecified
All
Android
relnote
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(p11+)

Details

(Whiteboard: [lang=java])

Attachments

(2 attachments, 1 obsolete attachment)

"android-app://" uris are shorthand for intent uris (bug 851693) with a definite package. These uris were introduced in API 22 (5.1).

Note that we should implement bug 851693 first because intent uris are more likely to be widespread and affect a larger range of users.

Updated

3 years ago
tracking-p11: --- → +
Assignee: nobody → michael.l.comella
Mentor: michael.l.comella
Created attachment 8606065 [details]
MozReview Request: bz://1162182/mcomella

/r/8805 - Bug 1162182 - Add feature22Plus to Versions. r=margaret
/r/8807 - Bug 1162182 - Handle android-app:// URIs. r=margaret

Pull down these commits:

hg pull -r 4fb3d55a906311a6ce16c4f0bd3e354cc29b55c7 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8606065 - Flags: review?(margaret.leibovic)

Comment 2

3 years ago
https://reviewboard.mozilla.org/r/8807/#review7557

This approach seems fine, but please consider my suggestions for making this code easier to follow.

::: mobile/android/base/GeckoAppShell.java:1273
(Diff revision 1)
> +    private static Intent getIntentSchemeIntent(final String targetURI,

This method name is a bit misleading, since it's not just for intent schemes.

I don't like boolean flags, so how about refactoring this to just handle checking the scheme itself? Maybe we could name it something like `getIntentFromURI`, which takes a URI string, and either returns an intent based on the scheme of that URI.

I suppose this would lead us to check the scheme twice, since we would only want to call this method for those special schemes, but I feel like that would be more straightforward than passing a boolean flag around.

Either that, or we should just have two helper methods - one for getting an Intent from an intent scheme, and one for getting an internet from an android-app scheme.

Comment 3

3 years ago
Comment on attachment 8606065 [details]
MozReview Request: bz://1162182/mcomella

https://reviewboard.mozilla.org/r/8803/#review7559

Ship It!
Attachment #8606065 - Flags: review?(margaret.leibovic) → review+
Feedback from a partner testing: 
" We think launching application using "android-app://" URIs are worked correctly. (Including the case that a receiver application does not exist.) "
Flags: needinfo?(michael.l.comella)
(In reply to Karen Rudnitski [:kar] from comment #4)
> Feedback from a partner testing: 
> " We think launching application using "android-app://" URIs are worked
> correctly. (Including the case that a receiver application does not exist.) "

I noticed this in my local testing, but I can't tell you why - we have no code to handle this specific case in the Android way [1], nor do we even seem to handle "android-app" urls in any special way [2].

I'd feel safer landing this patch (once the latest Android build tools can be pushed to the builders), but it will likely break our flow when an app is not installed until bug 1168980 lands).

[1]: https://mxr.mozilla.org/mozilla-central/search?string=parseUri&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
[2]: https://mxr.mozilla.org/mozilla-central/search?string=android-app&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Flags: needinfo?(michael.l.comella)
I'm going to hold off landing this until it's fully working because it affects a small amount of our users (i.e. just Android 5.1+) and if it's automagically working, then so be it.
Comment on attachment 8606065 [details]
MozReview Request: bz://1162182/mcomella
Attachment #8606065 - Attachment is obsolete: true
Attachment #8620241 - Flags: review+
Attachment #8620242 - Flags: review+
Created attachment 8620241 [details]
MozReview Request: Bug 1162182 - Add feature22Plus to Versions. r=margaret
Created attachment 8620242 [details]
MozReview Request: Bug 1162182 - Handle android-app:// URIs. r=margaret
Let's wait for bug 1168980 too.
Depends on: 1168980

Updated

3 years ago
Keywords: relnote
Closing as this will be properly implemented in:
  * FF41: bug 1182328
  * FF42: bug 1182140
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1182140
You need to log in before you can comment on or make changes to this bug.