New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 806654 link

Starred by 7 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 740783
issue 814123



Sign in to add a comment

desktop-pwas: window.open() to a cross origin URL from an app window leaves a blank app window opened

Project Member Reported by ortuno@chromium.org, Jan 29 2018

Issue description

Steps to reproduce:

1. Install https://pokedex.org by clicking the three dot menu and clicking Install to shelf...
2. Launch the app.
3. Open Dev Tools and run this code:

window.open('https://www.google.com', '_blank', 'height=200,width=150,toolbar=0,menubar=0,location=0');

Expected:

New tab or popup should open.

What happens?

A new blank app window and a new tab open.


We detect that an out-of-scope navigation is happening inside an app window and therefore open a new tab. The problem is that we don't close the opened window. We should change the code to also close the blank app window.
 

Comment 1 by mgiuca@chromium.org, Mar 26 2018

Labels: M-67

Comment 2 by mgiuca@chromium.org, Mar 27 2018

Labels: -Pri-3 Pri-2
I think this is something you're looking at now, with your popup analysis, Gio?

Dropping to Pri-2 since it's a fairly bad experience when it happens.

Comment 3 by ortuno@chromium.org, Mar 27 2018

Cc: hwi@chromium.org ortuno@chromium.org
 Issue 814124  has been merged into this issue.

Comment 4 by ortuno@chromium.org, Mar 27 2018

Blocking: 814123

Comment 5 by mgiuca@chromium.org, Mar 27 2018

Issue 796519 has been merged into this issue.

Comment 6 by mgiuca@chromium.org, Mar 27 2018

 Issue 814123  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e3890eb140383d6e7068fa1371ebe67edca8fe4

commit 0e3890eb140383d6e7068fa1371ebe67edca8fe4
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Apr 13 01:55:29 2018

desktop-pwas: Stop opening out-of-scope popups in the app's theme

Moves out-of-scope initial navigations, which are popups from one of the
app's windows, into regular popups without the parent's theme and icon.

Bug:  806654 
Change-Id: If679a6ecee79068e94975ae317126ac3c05f9ba5
Reviewed-on: https://chromium-review.googlesource.com/1011463
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550469}
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_experimental_navigation_throttle.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.h
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/tools/metrics/histograms/enums.xml

Comment 8 by ortuno@chromium.org, Apr 13 2018

Status: Fixed (was: Assigned)

Comment 9 by mgiuca@chromium.org, Apr 16 2018

This didn't make it into branch.

CL is +126 so might be difficult to merge. Gio, what do you think? I think it would be good to have this bug fixed.
+1 The current experience is pretty bad IMO.
Also, the actual changes are less than 50 lines. The rest is test code.
OK. Canary isn't updated yet. Will wait.
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e3890eb140383d6e7068fa1371ebe67edca8fe4

commit 0e3890eb140383d6e7068fa1371ebe67edca8fe4
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Apr 13 01:55:29 2018

desktop-pwas: Stop opening out-of-scope popups in the app's theme

Moves out-of-scope initial navigations, which are popups from one of the
app's windows, into regular popups without the parent's theme and icon.

Bug:  806654 
Change-Id: If679a6ecee79068e94975ae317126ac3c05f9ba5
Reviewed-on: https://chromium-review.googlesource.com/1011463
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550469}
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_experimental_navigation_throttle.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.h
[modify] https://crrev.com/0e3890eb140383d6e7068fa1371ebe67edca8fe4/tools/metrics/histograms/enums.xml

Labels: -merge-merged-testbranch Merge-Request-67
Status: Verified (was: Fixed)
Verified on 68.0.3397.0
Project Member

Comment 15 by sheriffbot@chromium.org, Apr 20 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Dev/Beta Release. Thank you.
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 20 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19359b8c7351d6510f83af3a18653480e5dd898a

commit 19359b8c7351d6510f83af3a18653480e5dd898a
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Apr 20 23:33:56 2018

desktop-pwas: Stop opening out-of-scope popups in the app's theme

Moves out-of-scope initial navigations, which are popups from one of the
app's windows, into regular popups without the parent's theme and icon.

Bug:  806654 
Change-Id: If679a6ecee79068e94975ae317126ac3c05f9ba5
Reviewed-on: https://chromium-review.googlesource.com/1011463
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550469}(cherry picked from commit 0e3890eb140383d6e7068fa1371ebe67edca8fe4)
Reviewed-on: https://chromium-review.googlesource.com/1023110
Cr-Commit-Position: refs/branch-heads/3396@{#185}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/chrome/browser/extensions/bookmark_app_experimental_navigation_throttle.cc
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.h
[modify] https://crrev.com/19359b8c7351d6510f83af3a18653480e5dd898a/tools/metrics/histograms/enums.xml

Sign in to add a comment