1198369 - If they cannot be opened, Intent URIs should go to fallback url before searching the play store

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

As reported in bug 851693 comment 48 (and subsequent research), we do the opposite!

https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/IntentHelper.java?rev=a5d7ee79a10c#204

Comment 1

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

Note that Chrome does the expected behavior.

Comment 2

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

Attachment: MozReview Request: Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

This behavior mimics Chrome's.

Comment 3

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

statenderrick, my patch fixes the issues as I outlined in bug 851693 but does not entirely work with your sample test case. Specifically, my patch opens the fallback url (rather than the store) but then opens an error page ("The address was not understood"). I added this error page to be more helpful to users when intent uris are incorrect, much like the error pages users will see when they click a bad link ("Server not found").

Looking through the sample page, I found:

<script type="text/javascript">
  window.onload = function() {
    window.location = "ihr://...";
  }
</script>

I assume this is meant to open the app if it exists, otherwise do nothing. When chrome receives an unknown Intent URI request, they seem to drop it, whereas we now display the error page.

In another bug 1189957, we found a another web property that did this, however, they used a hidden iframe for loading the url in the background. If it's an intent uri, it'll open the application as expected, but the error page will load in the hidden iframe, causing no interruption to the user if the application is not installed. From my understanding, I think the easiest fix would be to switch to this behavior.

If you think the error page behavior is undesirable (or incorrect), let me know and we'll consider changing it! In any case, if enough sites run into this issue, Firefox may seem broken to users and we may need to conform anyway.

Comment 4

[statenderrick]

Hi Michael,

Thank you! Do you know when this will be available in the beta version on the Play Store? Or is there a way for me to download an apk and test now?

We fire URI schemes in iframes most of the time -- Firefox pre-41 is actually one of the rare exceptions. This is legacy code that works for Firefox 40 and below and will not be served up for Firefox 41.

Once I can test I can follow up if there is anything we cannot work around on our end. I'm super excited for this!

Thanks again,
Derrick

Comment 5

[:Margaret Leibovic]

Comment on attachment 8652553 [details]
MozReview Request: Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

https://reviewboard.mozilla.org/r/17179/#review15439

Doh, good catch!

Comment 6

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

(In reply to statenderrick from comment #4)
> Thank you! Do you know when this will be available in the beta version on
> the Play Store? Or is there a way for me to download an apk and test now?

This patch should be in tomorrow's Nightly, which you can download from [1].

It's unclear when this will be in beta – I'll probably try to get it into the next release channel (Aurora), pending review, which will move to Beta on September 22nd-ish (for release in early November).

[1]: https://nightly.mozilla.org/

Comment 7

[statenderrick]

Great! I'll make a note to look on Friday.

Comment 8

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

(In reply to Michael Comella (:mcomella) from comment #6)
> This patch should be in tomorrow's Nightly, which you can download from [1].

It seems the tree is closed so this will be the case if I can push soon, otherwise it will be one of the following days – I'll let you know.

Comment 9

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

url:        https://hg.mozilla.org/integration/fx-team/rev/b32b5a757f627853f146e023aa9bb2b9b8d79905
changeset:  b32b5a757f627853f146e023aa9bb2b9b8d79905
user:       Michael Comella <[e-mail]>
date:       Tue Aug 25 13:56:29 2015 -0700
description:
Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

This behavior mimics Chrome's.

Comment 10

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

bug 851693 landed in 41 – let's see if we can get this in 41.

Comment 11

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

By the way, statenderrick, I'd expect this change to make it into tomorrow's (i.e. Friday morning's) Nightly.

Comment 12

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

Comment on attachment 8652553 [details]
MozReview Request: Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

Approval Request Comment
[Feature/regressing bug #]: bug 851693 and its followups (see bug 1182695 for the full saga)

[User impact if declined]:
  Websites use Chrome as the standard for how Intent URIs should work. Our previous behavior did not mimic Chrome to the letter but rather would search the Play Store if a non-installed application package was given in the Intent URI before opening the provided fallback URL. Chrome does the opposite – it provides the fallback url and then searches with the info in the Intent URI.
  Since websites have already designed themselves around Chrome's behavior, it is possible their flows do not work as expected on Firefox (and require additional compatibility efforts, e.g. bug 851693 comment 48 and subsequent comments). This patch fixes this issue and makes Firefox act like Chrome.

[Describe test coverage new/current, TreeHerder]: Tested locally
[Risks and why]: 
  Low – we reverse the order that two already-working if statements are called at the end of the function

[String/UUID change made/needed]: None

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[statenderrick]

> Target Milestone: --- → Firefox 43

Does this mean it will not be included in Firefox 41? Please let me know. This would be a significant setback for several hundred out of the thousands of apps that use the Branch SDK. Or perhaps I'm misunderstanding Target Milestone.

Comment 15

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

(In reply to statenderrick from comment #14)
> > Target Milestone: --- → Firefox 43
> 
> Does this mean it will not be included in Firefox 41?

Target milestone indicates which release the patches initially land on.

I've flagged the patch for uplift ("approval-mozilla-beta/aurora?") so depending on if I get approval for aurora (42) and the beta (41), just one, or none, it'll be released in those versions. It's an uncomplex change and we have just under a month until merge so I'd expect to get it in both.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8652553 [details]
MozReview Request: Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

Let's take in aurora as it improves a new feature. Ritu will make the call for beta.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/84dbc3d5110a

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

statenderrick, based on https://bugzilla.mozilla.org/show_bug.cgi?id=851693#c48, could I request you to verify this fix works as expected on the latest Nightly build 8/28? Thanks.

Comment 19

[statenderrick]

This does indeed work as expected. Thank you. I really appreciate that you all are so responsive.

It appears this is fixed in 42, correct? When does 42 go live?

Comment 20

[statenderrick]

I see September 22 is target date. Thanks all.

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to statenderrick from comment #20)
> I see September 22 is target date. Thanks all.

First off, many thanks for the verification! This fix should be in FF41 which will go live on Sept 22.

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8652553 [details]
MozReview Request: Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

The fix was verified which is awesome! Seems safe to uplift to Beta41.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d295d69ce116

Comment 24

[Catalin Suciu [:csuciu]]

Loading the test case from https://bugzilla.mozilla.org/show_bug.cgi?id=851693#c50 :
 - on beta 4, the Play Store is launched 
 - on beta 6, it falls back to bnc.lt for 1 sec, then redirects to the about:neterror page.
From comment #3, I understand that this is the expected behaviour.

Verifying as fixed on FF 41.0b6.