mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.27k forks source link

Setting to choose how to open app intents #7099

Closed bifleming closed 4 years ago

bifleming commented 4 years ago

User Story

Dependencies

mozilla-mobile/android-components#4950 Meta: #3177

Strings provided by Content Strategy

[label] Open links in apps [microcopy] Use external apps (like Maps or YouTube) when possible

Acceptance Criteria

┆Issue is synchronized with this Jira Task

liuche commented 4 years ago

Seems like @rocketsroger will be doing all of this and handling the rest of the edge cases (with help from Fenix for frontend if needed).

sblatz commented 4 years ago

@rocketsroger mentioned that this is waiting on https://github.com/mozilla-mobile/fenix/issues/5664

sblatz commented 4 years ago

@rocketsroger Are you able to provide an update on this?

rocketsroger commented 4 years ago

@rocketsroger Are you able to provide an update on this?

Unfortunately no updates. This issue requires a custom sheet to work as designed.

sblatz commented 4 years ago

Mocks here: https://app.abstract.com/share/1a68a45a-52be-4576-b0f7-e85b6ed73834?mode=design&sha=e90a0086037e564b9c8745b9e8cd1f4df40a6e01

sblatz commented 4 years ago

According to Roger this is something we can now do on the Fenix side. It should line up with the GV/AC API implementation. Let's take this over as a team.

Please touch base with him if you run into any issues.

BranescuMihai commented 4 years ago

@liuche @rocketsroger short clarification: So basically this story is more or less adding a UI toggle that is the opposite of the one we now have in Settings ("Open links in apps")?

Also, what is the expected behaviour when the toggle is on, so we prevent 3rd party apps, however our browser does not support the protocol, such as 'mailto'? Right now, we get an 'Unknown protocol' error. Maybe we should check if our browser can handle it, and if not, then redirect to 3rd party app regardless of that toggle.

Sidenote, maybe a bug: With the current implementation ('Open links in apps' set to on), most app deeplinks redirect us to the Store, however for Tinder, there seems to be another layer of redirecting, which results in the same 'Unknown protocol' error. This might be an edge-case that we should handle.

vesta0 commented 4 years ago

@BranescuMihai I don't think this story is the opposite of what we have now. In fact, I think this story is almost done and working as intended with the exception of the issues you already correctly called out:

@sblatz and @rocketsroger I assume #5664 is blocking the above 2 use cases?

vesta0 commented 4 years ago

I just saw the mocks posted above and now I understand where the confusion was. I think we should stick with the existing setting toggle and just make sure that the 2 issues I mentioned above are resolved.

rocketsroger commented 4 years ago

I just saw the mocks posted above and now I understand where the confusion was. I think we should stick with the existing setting toggle and just make sure that the 2 issues I mentioned above are resolved.

Understood. I'll investigate and raise issue(s) if its not related to https://github.com/mozilla-mobile/fenix/issues/5664 or https://bugzilla.mozilla.org/show_bug.cgi?id=1600704. Thanks,

rocketsroger commented 4 years ago
* Opening a play store link still doesn't work in many cases and seems to be more than just an edge case. I get the "Unknown Protocol" error page in all the following examples: Tinder, Facebook, Pinterest, Lyft, Uber, Pocket, and many more. 

This is confirmed to be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1600704

* Regression on opening `mailto` and `tel:`. This used to work in previous versions.

Can't reproduce this issue. It works for me but requires to turn on Open links in apps. Is this an expected behavior? or should we always launch mailto and tel links regardless of the user settings?

rocketsroger commented 4 years ago
* Regression on opening `mailto` and `tel:`. This used to work in previous versions.

Opened an issue to track this https://github.com/mozilla-mobile/android-components/issues/5892

rocketsroger commented 4 years ago

Confirmed with fixes from https://github.com/mozilla-mobile/android-components/issues/5892, https://github.com/mozilla-mobile/android-components/issues/5407 and https://bugzilla.mozilla.org/show_bug.cgi?id=1600704 the two issues @vesta0 stated have been fixed. I'll wait for nightly before requesting QA verification.

rocketsroger commented 4 years ago
* Opening a play store link still doesn't work in many cases and seems to be more than just an edge case. I get the "Unknown Protocol" error page in all the following examples: Tinder, Facebook, Pinterest, Lyft, Uber, Pocket, and many more.

* Regression on opening `mailto` and `tel:`. This used to work in previous versions.

Please verify these two issues are fixed on nightly. Thanks,

Xefir commented 4 years ago

Sadly, seems like that not all cases is fixed. I succeeded to reproduce this bug (https://github.com/mozilla-mobile/fenix/issues/8880) myself with the authentication of the Tusky app.

For testing, install Tusky and try to log-in. It open Firefox and ask for Authorize, but after confirming, a "Unknown Protocol" page is displayed instead of going back to the app.

I tested on the latest Firefox Fenix Preview Nightly on Play Store. Thank you for your work though.

rocketsroger commented 4 years ago

For testing, install Tusky and try to log-in. It open Firefox and ask for Authorize, but after confirming, a "Unknown Protocol" page is displayed instead of going back to the app.

We have a issue tracking this. https://github.com/mozilla-mobile/fenix/issues/7257. This will require more fixes from GeckoView.

rocketsroger commented 4 years ago
* Opening a play store link still doesn't work in many cases and seems to be more than just an edge case. I get the "Unknown Protocol" error page in all the following examples: Tinder, Facebook, Pinterest, Lyft, Uber, Pocket, and many more.

* Regression on opening `mailto` and `tel:`. This used to work in previous versions.

Confirmed both cases are now fixed. Please verify so this can be closed. Thanks,

lobontiumira commented 4 years ago

Verified as fixed on the latest Nightly build from 3/5 with Samsung Galaxy Note 8 (Android 9):