Closed Bug 1198369 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox41 --- verified
firefox42 --- fixed
firefox43 --- verified
fennec 41+ ---

People

(Reporter: mcomella, Assigned: mcomella)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Note that Chrome does the expected behavior.
Bug 1198369 - Go to fallback url before searching store for Intent handler. r=margaret

This behavior mimics Chrome's.
Attachment #8652553 - Flags: review?(margaret.leibovic)
Assignee: nobody → michael.l.comella
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.
Flags: needinfo?(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
Flags: needinfo?(statenderrick)
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!
Attachment #8652553 - Flags: review?(margaret.leibovic) → review+
(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/
Great! I'll make a note to look on Friday.
(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.
url:        https://hg.mozilla.org/integration/fx-team/rev/b32b5a757f627853f146e023aa9bb2b9b8d79905
changeset:  b32b5a757f627853f146e023aa9bb2b9b8d79905
user:       Michael Comella <michael.l.comella@gmail.com>
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.
bug 851693 landed in 41 – let's see if we can get this in 41.
tracking-fennec: --- → 41+
By the way, statenderrick, I'd expect this change to make it into tomorrow's (i.e. Friday morning's) Nightly.
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
Attachment #8652553 - Flags: approval-mozilla-beta?
Attachment #8652553 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b32b5a757f62
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
> 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.
Flags: needinfo?(michael.l.comella)
(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.
Flags: needinfo?(michael.l.comella)
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.
Attachment #8652553 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Flags: needinfo?(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?
Flags: needinfo?(statenderrick)
I see September 22 is target date. Thanks all.
(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 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.
Attachment #8652553 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
You need to log in before you can comment on or make changes to this bug.