1389236 - Open external links in PWAs in a Custom Tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), enhancement)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Instead of doing a mini-browser as in bug 1388747, product has requested we simply kick out to a custom tab, since that's pretty much what you'd expect in a native app as well.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Eugen, this might be the next thing to tackle, as discussed. We'll need to not only handle target=_blank stuff, but also links that are in the same window.

Comment 2

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1389236-1.0-Allow-sync-handling-of-all-URI-loadi.patch

Wait for the callback result for each URI loading request.
The result is boolean for now. The handler returns true if the request has been handled, false if Gecko should handle it instead. In the latter case, GeckoView decided on how to handle it.

I've also simplified the "where" argument, possible target windows are now DEFAULT, CURRENT and NEW, removing NEWTAB and SWITCHTAB which are specific to Fennec.

Comment 3

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1389236-2.0-Mark-URI-loading-requests-in-the-cus.patch

Custom tabs always return true since we always handle the event.

Comment 4

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1389236-3.0-Launch-a-custom-tabs-intent-for-out-.patch

PWAs launch a custom tabs intent for URIs not in their scope.
This will fire the generic custom tabs intent.

Comment 5

[Eugen Sawin [:esawin]]

Attachment: 0004-Bug-1389236-4.0-Mark-URI-loading-requests-in-the-Gec.patch

Comment 6

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1389236-1.0-Allow-sync-handling-of-all-URI-loadi.patch

Fixed syntax error.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8897009 [details] [diff] [review]
0003-Bug-1389236-3.0-Launch-a-custom-tabs-intent-for-out-.patch

Review of attachment 8897009 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
@@ +270,4 @@
>          if (isInScope(uri)) {
>              view.loadUri(uri);
>          } else {
> +            CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();

I think we should use the theme_color from the manifest as the toolbar color.

@@ +271,5 @@
>              view.loadUri(uri);
>          } else {
> +            CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
> +            CustomTabsIntent intent = builder.build();
> +            intent.launchUrl(this, Uri.parse(uri));

I think we may need to do our own thing here to ensure that we launch into our own Custom Tab implementation and not whatever it chooses. Maybe it's enough to set the class on the CustomTabsIntent.intent instance?

Comment 8

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1389236-3.1-Launch-a-custom-tabs-intent-for-out-.patch

Launch our custom tabs activity for out-of-scope URI load requests.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8898840 [details] [diff] [review]
0003-Bug-1389236-3.1-Launch-a-custom-tabs-intent-for-out-.patch

Review of attachment 8898840 [details] [diff] [review]:
-----------------------------------------------------------------

We should set the theme color too, but maybe that can be done in a followup.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fe42a7f6174
[1.0] Allow sync handling of all URI loading requests in the app. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/8013338c457c
[2.0] Mark URI-loading requests in the custom tabs handler as handled. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/dac0b47a5734
[3.1] Launch a custom tabs intent for out-of-scope URIs in the PWA's onLoadUri handler. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ee92d119955
[4.0] Mark URI-loading requests in the GeckoView example app handler as handled. r=snorp

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db59558afbdf
[5.0] Fix rebase. r=me

Comment 12

[Andreas Bovens [:abovens]]

>  We'll need to not only handle target=_blank stuff, but also links that are in the same window.

Yep, essentially, we want links that are outside of the PWA scope to open in a custom tab.

> I think we should use the theme_color from the manifest as the toolbar color.

That seems indeed like a good idea. To be clear: we want to use the theme_color asset defined in the manifest of the installed PWA, and not the theme color defined in the <meta> section or manifest of the site loaded in the custom tab.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1fe42a7f6174
https://hg.mozilla.org/mozilla-central/rev/8013338c457c
https://hg.mozilla.org/mozilla-central/rev/dac0b47a5734
https://hg.mozilla.org/mozilla-central/rev/8ee92d119955
https://hg.mozilla.org/mozilla-central/rev/db59558afbdf

Comment 14

[Oana Horvath [:ohorvath]]

Verified as fixed on Nightly 58 (2017-09-22).
Devices:
Prestigio Grace X5 (Android 4.4.2)
LG G4 (Android 5.1)
Huawei Honor 8 (Android 6.0)
Asus ZenPad 8.0 Z380KL (Android 6.0.1)