1218086 - Play store intent URI fails to load

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Try loading this URL: http://invite.ritual.co/GAVIN4555

In Nightly, we load about:neterror. In Chrome, it launches the play store.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I remember a similar situation encountered while working on the initial intent URI code. IIRC, that was by design.

Comment 2

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

The intent url we get from the page is:
  intent://open#Intent;scheme=ritualco;package=co.ritual.app;S.browser_fallback_url=market%3A%2F%2Fdetails%3Fid%3Dco.ritual.app%26referrer%3Dlink_click_id%253D188071668298611434?referrer=link_click_id%3D188071668298611434;end

Notably, the fallback url has the scheme `market`. In bug 1208956 we switched to only using http & https schemes in the fallback url, which I believe is also Chrome's behavior.

We automatically search the play store if a package is specified in the uri and will travel to fallback uris if they're specified. If both are specified, we defer to the fallback uri. I set this behavior to match Chrome. You can see on my intent URI test page [1] for both Firefox & Chrome, clicking the "Click here to open zxing with an intent:// URI and fallback url!" link (both package & fallback uri) will open the fallback url instead of opening the play store with the package.

My guess is that if the fallback uri is invalid, Chrome reverts to opening the play store if applicable, which we don't do. Seems easy enough to fix.

[1]: https://people.mozilla.org/~mcomella/test/uri.html

Comment 3

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

Attachment: MozReview Request: Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret

Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret

Comment 4

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

Worth noting that they do a redirect which acts differently in Firefox and Chrome after this patch. In Chrome, the play store will open and when you press back, it'll take you to Bugzilla. In Firefox, the play store will open and when you press back, it'll take you to their redirect page: https://bnc.lt/m/qgZOBscy2m

I'm not sure if this is the intent handling code's fault or what but you're welcome to file another bug if you think it's worth investigating.

Comment 5

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

(In reply to Michael Comella (:mcomella) from comment #4)
> you're welcome to file another bug if you think it's worth investigating.

yolo: bug 1219518.

Comment 6

[:Margaret Leibovic]

Comment on attachment 8680392 [details]
MozReview Request: Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret

https://reviewboard.mozilla.org/r/23623/#review21147

Nice job investigating. I like that test page!

::: mobile/android/base/IntentHelper.java
(Diff revision 1)
> -                urlToLoad = MALFORMED_URI_PREFIX + fallbackUrl;

Looks like GENERIC_URI_PREFIX and MALFORMED_URI_PREFIX aren't used anymore, so you should remove their declarations.

I'm happy to see us remove this logic to load about:neterror directly. That seemed fishy to me before, but I wasn't sure of how to avoid that.

Comment 7

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

https://reviewboard.mozilla.org/r/23623/#review21147

> Looks like GENERIC_URI_PREFIX and MALFORMED_URI_PREFIX aren't used anymore, so you should remove their declarations.
> 
> I'm happy to see us remove this logic to load about:neterror directly. That seemed fishy to me before, but I wasn't sure of how to avoid that.

> I'm happy to see us remove this logic to load about:neterror directly.

To be fair, we still load in in the case that the built-in `Intent.parseUri` throws, or if we don't manage to use a fallback url or open the play store.

Comment 8

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

https://hg.mozilla.org/integration/fx-team/rev/fba5a9a4ba80b74853753234a6f47d342abf0eb5
Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret

Comment 9

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

Let's put this in 44 because it's a good bug fix.

Comment 10

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

Comment on attachment 8680392 [details]
MozReview Request: Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret

Approval Request Comment
[Feature/regressing bug #]: Always existed since this was originally implemented (intent URIs in FF41, bug 851693, but I don't know when fallback uris landed).

[User impact if declined]:
  Users on edge case sites that provide invalid fallback uris will go to about:neterror, rather than trying to open the Play Store for valid links.

[Describe test coverage new/current, TreeHerder]: Tested locally.

[Risks and why]: 
  Low – we reorder existing logic to make this happen.

[String/UUID change made/needed]: None

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fba5a9a4ba80

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/efb000db02f6

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/fba5a9a4ba80

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/efb000db02f6

Comment 15

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

> [1]: https://people.mozilla.org/~mcomella/test/uri.html

My test page has been moved:
  https://mcomella.github.io/test/uri.html