Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 740402 PWA opens _blank links in PWA whern an external app (e.g. YouTube) is more suitable.
Starred by 2 users Project Member Reported by piotrs@chromium.org, Jul 9 Back to list
Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 709889



Sign in to add a comment
Reported in  crbug.com/709889  by nekr.fabula@gmail.com:

I tested this a bit further and it turns out that with this features added Chrome stopped opening external links other apps. For example twitter/youtube/google maps links previously would be opened in its native app  if the one is installed, but now everything is being opened in CCT.

This is clearly a regression to me.

(now sure how other browsers behave here, e.g. Safari, but iOS home screen webapps are so poor and bad so I'm sure if that is even a good example)

Also one more note (might be useful to know too):

Previously, only when one specify rel="noopener" on a link, it was opening the link Chrome. Example:

<a href="https://youtube.com/channel/..." target="_blank">Youtube</a> -- this would open native app. 

<a href="https://youtube.com/channel/..." target="_blank" rel="noopener">Youtube</a> -- this would NOT open a native app, but rather open the link in Chrome.

With CCT feature it always opens in CCT. I can file a separate bug for rel="noopener" if that makes sense. That might not be a bug though, I'm not sure, but I haven't found any specs/docs about such specific behavior.

 
Blocking: 709889
Piotr, thank you for filing the bug! Having a separate bug to track this work makes what is being fixed clearer (and will likely make my future self happier)
Project Member Comment 3 by bugdroid1@chromium.org, Jul 12
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e

commit 727468b41b2fe48bf73983ab0c3aa2632b47d89e
Author: piotrs <piotrs@chromium.org>
Date: Wed Jul 12 04:06:43 2017

Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#485831}

[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/java_sources.gni
[delete] https://crrev.com/28132e66abebde370e168fff9ff2fa157c979d55/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
[modify] https://crrev.com/727468b41b2fe48bf73983ab0c3aa2632b47d89e/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java

Status: Fixed
This should work now.
Project Member Comment 5 by bugdroid1@chromium.org, Jul 12
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f

commit a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f
Author: aberent <aberent@chromium.org>
Date: Wed Jul 12 10:03:45 2017

Revert of Fixes redirects to external apps when navigating from PWA. (patchset #9 id:280001 of https://codereview.chromium.org/2969143002/ )

Reason for revert:
Test failures from new test WebappNavigationTest#testNewTabLinkToExternalApp on hromium.android/Marshmallow 64 bit Tester.

BUG=741568

Original issue's description:
> Fixes the redirect to external apps when navigating from PWA.
>
> This is done with avoiding explicit chrome package name (using
> component name instead) and using
> EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.
>
> With this patch the redirect works, however it takes a long time for CCT to
> consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
> in ChromeTabbedActivity is almost instantaneous. Result is that
> for _blank links CCT shows up briefly before redirecting to an external app.
> In a follow up it should be investigated why it is the case and if it cannot
> be improved, we might need to bypass CCT in such case, which would diverge
> from existing navigation paths for _blank links in Clank.
>
> BUG=740402
>
> Review-Url: https://codereview.chromium.org/2969143002
> Cr-Commit-Position: refs/heads/master@{#485831}
> Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e

TBR=dominickn@chromium.org,yusufo@chromium.org,tedchoc@chromium.org,piotrs@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=740402

Review-Url: https://codereview.chromium.org/2975883003
Cr-Commit-Position: refs/heads/master@{#485910}

[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/java_sources.gni
[add] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
[modify] https://crrev.com/a1b6731c183e2273c3f3b013bf6db0bfe66fdb2f/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java

Project Member Comment 6 by bugdroid1@chromium.org, Jul 13
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f

commit 4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f
Author: piotrs <piotrs@chromium.org>
Date: Thu Jul 13 05:16:27 2017

Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}

[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/java_sources.gni
[delete] https://crrev.com/184a75bcf8dda55ceef7f6df67154ce822c7cc5d/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
[modify] https://crrev.com/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java

Project Member Comment 7 by bugdroid1@chromium.org, Jul 13
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7

commit b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7
Author: aberent <aberent@chromium.org>
Date: Thu Jul 13 10:18:13 2017

Revert of Fixes redirects to external apps when navigating from PWA. (patchset #10 id:300001 of https://codereview.chromium.org/2969143002/ )

Reason for revert:
Still failing on Marshmallow 64 bit Tester. See, for example https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%2064%20bit%20Tester/builds/13798

BUG=741568

Original issue's description:
> Fixes the redirect to external apps when navigating from PWA.
>
> This is done with avoiding explicit chrome package name (using
> component name instead) and using
> EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.
>
> With this patch the redirect works, however it takes a long time for CCT to
> consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
> in ChromeTabbedActivity is almost instantaneous. Result is that
> for _blank links CCT shows up briefly before redirecting to an external app.
> In a follow up it should be investigated why it is the case and if it cannot
> be improved, we might need to bypass CCT in such case, which would diverge
> from existing navigation paths for _blank links in Clank.
>
> BUG=740402
>
> Review-Url: https://codereview.chromium.org/2969143002
> Cr-Original-Commit-Position: refs/heads/master@{#485831}
> Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e
> Review-Url: https://codereview.chromium.org/2969143002
> Cr-Commit-Position: refs/heads/master@{#486275}
> Committed: https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f

TBR=dominickn@chromium.org,yusufo@chromium.org,tedchoc@chromium.org,piotrs@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=740402

Review-Url: https://codereview.chromium.org/2977023002
Cr-Commit-Position: refs/heads/master@{#486333}

[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/java_sources.gni
[add] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
[modify] https://crrev.com/b2fa8274b92136d9dd2d347b6e33b4e56bfe21f7/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java

Labels: -Pri-3 M-61 Pri-1
Status: Assigned
Looks like re-landign this will fix issue 742449 too. Would appreciate it if you can get this in for 61
Definitely will do everything to get this in and will test WebAPK -> CCT -> WebAPK case explicitly.
Project Member Comment 10 by bugdroid1@chromium.org, Jul 20 (6 days ago)
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/471df3926c09fda0af15d9157eb29bd326b0f9fc

commit 471df3926c09fda0af15d9157eb29bd326b0f9fc
Author: piotrs <piotrs@chromium.org>
Date: Thu Jul 20 13:41:20 2017

Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Original-Commit-Position: refs/heads/master@{#485831}
Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e
Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#486275}
Committed: https://chromium.googlesource.com/chromium/src/+/4dbcecfb630ac1a2dba1910f0e429b82bf3a0b5f
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#488228}

[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java
[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/java_sources.gni
[delete] https://crrev.com/9219f3033f0f56ec5477336b11915e7e2e9f46f2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
[modify] https://crrev.com/471df3926c09fda0af15d9157eb29bd326b0f9fc/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java

Comment 11 by piotrs@chromium.org, Jul 20 (6 days ago)
Status: Fixed
Hopefully the patch with stick this time and it will be fixed for good.
Comment 12 by piotrs@chromium.org, Jul 24 (2 days ago)
Labels: Merge-Request-61
I would like to merge commit 471df3926c09fda0af15d9157eb29bd326b0f9fc into m61, thanks!
Project Member Comment 13 by sheriffbot@chromium.org, Jul 24 (2 days ago)
Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Comment 14 by piotrs@chromium.org, Yesterday (25 hours ago)
Status: Started
Comment 15 by piotrs@chromium.org, Yesterday (25 hours ago)
Reason for Merge request:
Usage of CCT in standalone PWAs has only been added in m61, this final patch fixes the regression where we don't favor native specialized handlers (e.g. native YouTube or Google Maps apps). It would be a shame to bring this to users, though arguably it's not very severe issue. Up for your consideration.
Sign in to add a comment