1162182 - Allow loading android-app:// uris

Status: RESOLVED DUPLICATE of bug 1182140

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

"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.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 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/

Comment 2

[:Margaret Leibovic]

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

[:Margaret Leibovic]

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

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

Ship It!

Comment 4

[Karen Rudnitski [:kar]]

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.) "

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(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

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fc91ae033ec

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1162182 - Add feature22Plus to Versions. r=margaret

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1162182 - Handle android-app:// URIs. r=margaret

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Let's wait for bug 1168980 too.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Closing as this will be properly implemented in:
  * FF41: bug 1182328
  * FF42: bug 1182140