1168980 - Search app stores when opening an intent:// URI that doesn't have an appropriate app installed

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

(Might depend on 1164114)

This is what Chrome does and it appears to be a more user intuitive flow than our current, "App not installed. Search?" popup.

The store search intent can be generic.

Steps to show pop-up:
1) Go to https://people.mozilla.org/~mcomella/test/intent_uri.html
2) Click a "here" link without zxing installed

Comment 1

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

Anthony, what do you think of this?

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

+1

Comment 3

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

To track this backwards, we open the URIs from:

IntentHelper.open
 IntentHelper.handleMessage
(-> JS by "Intent:Open*" [1])

(branch one - context menus - no action to take here)
HelperApps.jsm HelperApps.launchUri
 browser.js ExternalApps.openExternal
  browser.js ExternalApps.init (this gets called from the context menu when long-pressing)

(branch two - page actions? - no action to take here)
HelperApps.jsm HelperApps._launchApp
 HelperApps.jsm App.launch
  browser.js _setUriForPageAction (maybe more? [2])

(branch three - where we need to take action)
ContentDispatchChooser.js ask [3]

To be continued...

[1]: https://mxr.mozilla.org/mozilla-central/search?string=intent%3Aopen&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
[2]: https://mxr.mozilla.org/mozilla-central/ident?i=launch&tree=mozilla-central&filter=mobile%2Fandroid
[3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ContentDispatchChooser.js#63

Comment 4

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

I realized from ContentDispatchChooser.js ask(), we can send the URI that failed to match any apps from Java and handle it there. Working on a patch...

Comment 5

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

To be clear, we want to handle this from Java so we can use Intent.parseUri and use methods like Intent.getPackage on the returned Intent (why search for a scheme when you can search for a package?).

Comment 6

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

Attachment: MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Bug 1168980 - WIP

Comment 7

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

The jump to the store is pretty abrupt so we may want to prompt the user in a more satisfactory way than our current toast (I bet that's why we have it now). I think a dialog would be more appropriate, ala bug 1173147.

Anthony, any thoughts? Or would you like to see a build first?

Comment 8

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

(In reply to Michael Comella (:mcomella) from comment #7)
> Anthony, any thoughts? Or would you like to see a build first?

(For the record, that app could be chrome though I'm not sure what they do in the case below)

This may only be abrupt if we can't match the app exactly (e.g. the install page for the barcode scanner app appears rather than a search screen).

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Let's test this without an interstitial first. I think we can help the user get to the Google Play Store right away. I think in most cases, the context is given by wherever the user is coming from anyway. Factoring in the power of the hardware back button, this makes the behavior more "forgivable"

Comment 10

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

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Bug 1168980 - WIP

Comment 11

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

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Still TODO:
* Final copy
* Test fallback URLs
* Figure out how to handle android-app:// URIs on < 5.1 devices (should we just use the fallback url)?

Comment 12

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

Anthony, can I get your input on copy? Below:
  1) If the URL we receive is empty, or we cannot parse it (two separate steps), I figured we should show a toast to the user and take no other action. My current copy is, "Invalid link". Any preferences?
  2) First we try to search the store and if we can't, we go to the fallback url that the link specifies. If one is not specified, I figured we should show a toast to the user and take no action. My current copy is, "Unable to handle link". Any preferences?

Comment 13

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

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Bug 1168980 - Search app store when there is no app to handle intent:// URI. r?margaret

Comment 14

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I have some questions!

(In reply to Michael Comella (:mcomella) from comment #12)
> Anthony, can I get your input on copy? Below:
>   1) If the URL we receive is empty, or we cannot parse it (two separate
> steps), I figured we should show a toast to the user and take no other
> action. 

If this is in our super toast, could we possibly offer a "try again" action? Would it help? This would alleviate some of the frustration for users.

> My current copy is, "Invalid link". Any preferences?

How about like "Can't open link" ?

>   2) First we try to search the store 

In this situation, is it obvious to the user that this is happening?

> and if we can't, we go to the fallback
> url that the link specifies. 
> If one is not specified, I figured we should
> show a toast to the user and take no action. My current copy is, "Unable to
> handle link". Any preferences?

I also wonder if we need to have separate messages here. In either case, we've aborted from the process and the user hasn't moved anywhere in the application. To the user (in either case), the link didn't "work".

Maybe we should just repeat the same error message until we can offer them a solution or some form of action (like search GPS, or try again).

Thoughts?

Comment 15

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

(In reply to Anthony Lam (:antlam) from comment #14)
> >   1) If the URL we receive is empty, or we cannot parse it (two separate
> > steps), I figured we should show a toast to the user and take no other
> > action. 
> 
> If this is in our super toast, could we possibly offer a "try again" action?
> Would it help? This would alleviate some of the frustration for users.

It's currently not a super toast. If the URL is empty or we cannot parse it, there's nothing we can do differently so there's nothing to try again. Unless I wrote a bug, this will usually be the web developer's fault and so we can only hope they have these links correct for their users.

> > My current copy is, "Invalid link". Any preferences?
> 
> How about like "Can't open link" ?

"Cannot open link"?

> >   2) First we try to search the store 
> 
> In this situation, is it obvious to the user that this is happening?

I should be clear - we don't search, but if the link provides a package ID, we can open the app page directly (e.g. link tries to open zxing specifically and I don't have it installed, so the zxing Store page opens). The app switches with a standard animation, and the store page pops up. The only time it's unintuitive is if an invalid package is given where the store's "Item not found." page is shown, with a "Retry" button at the bottom - there is no action bar or title so there is no context beyond the app switch animation (don't blink!). Try out [1] in Chrome to get a sense for how this works. I'm not sure we can handle this better though, because I don't think there's a way to get a response from the Store without opening it (though we could open the store page in the browser instead and hit this page [2]). I think this is another case of web developers needing to get their links correct.

But that gives me an alternative solution - we can open an error page (like clicking a broken web link on mobile or desktop). But if so, follow-up? This page could also provide the ability to search some content from the link (I'm not sure which) like when you navigate to an unknown server. It's more work that I'm not sure I have time for right now and I feel the toast is an *alright* solution.

> > and if we can't, we go to the fallback
> > url that the link specifies. 
> > If one is not specified, I figured we should
> > show a toast to the user and take no action. My current copy is, "Unable to
> > handle link". Any preferences?
> 
> I also wonder if we need to have separate messages here. In either case,
> we've aborted from the process and the user hasn't moved anywhere in the
> application. To the user (in either case), the link didn't "work".

"Cannot open link" wfm in all cases.

> Maybe we should just repeat the same error message until we can offer them a
> solution or some form of action (like search GPS, or try again).

I don't think there's any solutions beyond searching (see above) but doing this in a super toast sucks (i.e. what we had) because it's unclear what you're searching for and most of the time, nothing good will come out of it.

[1]: https://people.mozilla.org/~mcomella/test/intent_uri.html
[2]: https://play.google.com/store/apps/details?id=lol.meh.haha

Comment 16

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

"Can't open links" has a more human tone to it. I prefer that one :)

Comment 17

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Let's go with "Cannot open link" right now. 

There may still be frustration and some element of "blame" that we can defer here. But, without offering the user a solution or some "action" to take, it's hard to really improve the situation I think.

Since it is not that common to come across this anyways, let's not block on cleaning up the copy.

Comment 18

[:Margaret Leibovic]

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

https://reviewboard.mozilla.org/r/10685/#review9645

This looks fine to me. Do you have a wiki page or something where you've been documenting how our intent handling works? There are so many various details you're learning in all these bugs, and it would be good for them to be in a central place that's not your brain :)

::: mobile/android/base/IntentHelper.java:28
(Diff revision 3)
> +import ch.boye.httpclientandroidlib.util.TextUtils;

Did you mean to import android.text.TextUtils?

::: mobile/android/base/IntentHelper.java:144
(Diff revision 3)
> +            Toast.makeText(activity, errText, Toast.LENGTH_LONG).show();

Maybe we should factor this error toast logic into a helper method.

::: mobile/android/components/ContentDispatchChooser.js:52
(Diff revision 3)
> -      let win = this._getChromeWin();
> +      let msg = {

You could also get rid of this `_getChromeWin` helper method.

Mak

Comment 19

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #18)

> Mak

(I don't know what this is about. I blame MozReview.)

Comment 20

[:Margaret Leibovic]

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

https://reviewboard.mozilla.org/r/10685/#review9649

Ship It!

Comment 21

[:Margaret Leibovic]

This also might be worth a release note.

Comment 23

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

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Comment 24

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

Attachment: MozReview Request: Bug 1168662 - Modify all outgoing Intents with CATEGORY_BROWSABLE. r=margaret

Bug 1168662 - Modify all outgoing Intents with CATEGORY_BROWSABLE. r=margaret

Comment 25

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

Attachment: MozReview Request: Bug 1168662 - Add EXTRA_APPLICATION_ID to all outgoing Intents. r=margaret

Bug 1168662 - Add EXTRA_APPLICATION_ID to all outgoing Intents. r=margaret

Comment 26

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

Comment on attachment 8617701 [details]
MozReview Request: Bug 1168980 - Search app store when there is no app to handle intent:// URI. r=margaret

Reviewboard fail.

Comment 27

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

https://hg.mozilla.org/integration/fx-team/rev/cf876b6ac843

Comment 28

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/2271296e404a for crashing the xpcshell tests netwerk/test/unit/test_redirect-caching_failure.js  and netwerk/test/unit/test_redirect_failure.js, https://treeherder.mozilla.org/logviewer.html#?job_id=3466782&repo=fx-team

Comment 29

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

Spoke w/ jchen on IRC who said xpcshell tests do not have an Android bridge, causing the failure. He mentioned I can check for the window object as a short-circuit because xpcshell tests also do not have a window object. Here's a try push (with the failing suite, X2, passing):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70f9a9619d20

Comment 30

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

https://hg.mozilla.org/integration/fx-team/rev/9bae6f3fdfb2

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9bae6f3fdfb2

Comment 33

[Sylvestre Ledru [:Sylvestre]]

Just like in bug 851693:
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:

Comment 34

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

This relnote seems like it'd step on bug 851693's toes too much (it's an implementation detail of the same feature) so I'll let Margaret decide if it's necessary and fill out the note.

Comment 35

[:Margaret Leibovic]

(In reply to Michael Comella (:mcomella) from comment #34)
> This relnote seems like it'd step on bug 851693's toes too much (it's an
> implementation detail of the same feature) so I'll let Margaret decide if
> it's necessary and fill out the note.

Good call, I think that the note in bug 851693 would cover this as well.