Closed Bug 1218086 Opened 4 years ago Closed 4 years ago

Play store intent URI fails to load

Categories

(Firefox for Android :: General, defect)

35 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 45
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed
fennec 44+ ---

People

(Reporter: Margaret, Assigned: mcomella)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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

In Nightly, we load about:neterror. In Chrome, it launches the play store.
Flags: needinfo?(michael.l.comella)
I remember a similar situation encountered while working on the initial intent URI code. IIRC, that was by design.
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
Assignee: nobody → michael.l.comella
Flags: needinfo?(michael.l.comella)
Bug 1218086 - Continue parsing Intent if fallback uri is invalid. r=margaret
Attachment #8680392 - Flags: review?(margaret.leibovic)
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.
(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 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.
Attachment #8680392 - Flags: review?(margaret.leibovic) → review+
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.
Let's put this in 44 because it's a good bug fix.
tracking-fennec: --- → 44+
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
Attachment #8680392 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/fba5a9a4ba80
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Attachment #8680392 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.