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

Issue 660496 link

Starred by 14 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , All , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

window.opener navigation inside a beforeunload/unload handler is ignored

Reported by m...@hanleym.com, Oct 28 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36

Steps to reproduce the problem:
1. Open index.html with popups enabled.
2. Close the popup (popup.html) that is opened by index.html.

What is the expected behavior?
The original window (opener) location should change to unload.html.

What went wrong?
The setting of window.opener.location.href was ignored and the original window (opener) location remained index.html.

Did this work before? Yes 414582

Does this work in other browsers? N/A

Chrome version: 54.0.2840.71  Channel: stable
OS Version: OS X 10.11.6
Flash Version: Shockwave Flash 23.0 r0

I am mostly confident that 8fb70277dba468aac9d2eae51e432d76667a79db (https://chromium.googlesource.com/chromium/src/+/8fb70277dba468aac9d2eae51e432d76667a79db) is the cause of this issue, however, the result of bisect-builds.py is below.

Additionally, the discussion referenced in the commit (https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/VfItzNe3WO0) states that this is consistent with the behavior of Firefox and Edge.  This is not the case with regards to window.opener as both allow this.

Finally, I think this bug (https://bugs.chromium.org/p/chromium/issues/detail?id=660276) is likely caused by the same issue.

bisect-builds.py:

You are probably looking for a change made after 414582 (known good), but no later than 414590 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/cbdfa25c7526a8ddddba3fc6a84c17b25263d48e..aaad992eef8e8c1e879f72afd9dabb05d3855e67
 
index.html
215 bytes View Download
popup.html
299 bytes View Download
unload.html
95 bytes View Download
Owner: lfg@chromium.org
Status: Assigned (was: Unconfirmed)

Comment 2 by lfg@chromium.org, Nov 2 2016

Cc: lfg@chromium.org
 Issue 660276  has been merged into this issue.

Comment 3 by lfg@chromium.org, Nov 3 2016

Cc: -lfg@chromium.org
Status: Started (was: Assigned)

Comment 4 by lfg@chromium.org, Nov 3 2016

This was unintentional, the idea is to block navigations only on the document that's being unloaded, I'll work on a fix, but a possible workaround you can use is:

window.onbeforeunload = function() {
  setTimeout(function() {
               window.opener.location.href = "something";
             }, 0);
}

I have the same problem. When do you think this issue will be fixed?

Comment 6 by lfg@chromium.org, Nov 8 2016

Labels: M-55
Re#5: Targeting M55 for a fix.

Comment 7 by lfg@chromium.org, Nov 8 2016

Labels: ReleaseBlock-Stable OS-Android OS-Chrome OS-Linux OS-Windows OS-All
I have the same issue. I tried the workaround suggested by lfg@chromium.org from this comment https://bugs.chromium.org/p/chromium/issues/detail?id=660496#c4, but no success. 
Is there any other possible workaround/quickfix?
We did something similar to the suggested fix and it works great:

window.onunload = function() {
 window.opener.callRefresh();
}; 

function callRefresh(){
	var winTimer = window.setInterval(function()
			{
			    window.clearInterval(winTimer);
			    location.href = your url go here
			}, 500);
}

Hope this helps.

Comment 10 by lfg@chromium.org, Nov 9 2016

Cc: creis@chromium.org lfg@chromium.org toyoshim@chromium.org
 Issue 661905  has been merged into this issue.
 Issue 664470  has been merged into this issue.
**** Bulk edit -  please ignore if not applicable ****


A friendly reminder that M55 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!

Also due to Thanksgiving holidays in US, please make sure fix is ready and merged to M55 latest by 5:00 PM PT Friday, 11/18/16 (sooner the better).

Comment 13 by lfg@chromium.org, Nov 15 2016

Cc: dcheng@chromium.org
Labels: -M-55 M-56
I talked with dcheng@ yesterday and we are moving this to M56. We are too close to merge this back to M55, and since there are plenty of workarounds available I don't think we should take the risk merging this to M55.

If you want to try the WIP patch, the CL is at https://codereview.chromium.org/2487403002/ .

Specifically, I'm worried about merging this because of the behavior changes for beforeunload: I haven't managed to convince myself that it's safe to let beforeunload handlers navigate *other* frames, even if those frames aren't in the subtree that will be navigated.
Cc: tkonch...@chromium.org hdodda@chromium.org
 Issue 651001  has been merged into this issue.
Any updates?
Components: Blink>Loader
This issue is tagged as Stable Blocker, can we have the latest update?

Comment 19 by lfg@chromium.org, Dec 12 2016

Review in progress, hoping to land a fix soon.
Any updates?
Gentle Ping... !
Any update on this stable blocker issue?

Thanks,

Comment 22 by lfg@chromium.org, Jan 9 2017

Cc: brajkumar@chromium.org
 Issue 679203  has been merged into this issue.
Project Member

Comment 23 by bugdroid1@chromium.org, Jan 9 2017

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

commit 2267cfcce06c92602376754343e5f45effeee1f1
Author: lfg <lfg@chromium.org>
Date: Mon Jan 09 21:27:48 2017

Allow navigations to frames that aren't being unloaded in the unload handler.

This change lets the page navigate frames that aren't being unloaded in
the unload handler. For example, a popup may want to navigate its
opener while it's being closed.

FrameLoader now also correctly checks if the |targetFrame| can be
navigated, instead of checking if |m_frame| can be navigated.

BUG= 660496 

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

[add] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/LayoutTests/http/tests/navigation/resources/targeted-navigation-in-unload-handler-subframe.html
[add] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/LayoutTests/http/tests/navigation/targeted-navigation-in-unload-handler.html
[modify] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/Source/core/frame/History.cpp
[modify] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
[modify] https://crrev.com/2267cfcce06c92602376754343e5f45effeee1f1/third_party/WebKit/Source/core/loader/NavigationScheduler.h

Labels: Needs-Feedback
Tested the issue on chrome latest M57-57.0.2977.0 Windows-10, Ubuntu 14.04 and Mac OS 10.12.2 and able to reproduce the issue.Please find the screen shot for reference.

@lfg: Could you please look in to this issue.

Thank You!
660496.PNG
161 KB View Download

Comment 25 by lfg@chromium.org, Jan 10 2017

Status: Fixed (was: Started)
I just tested and the issue is fixed on 2977. Make sure you have popups enabled if you are using the test case on comment #1.

Comment 26 by lfg@chromium.org, Jan 10 2017

Labels: -Needs-Feedback Merge-Request-56
Project Member

Comment 27 by sheriffbot@chromium.org, Jan 10 2017

Labels: -Merge-Request-56 Hotlist-Merge-Approved Merge-Approved-56
Your change meets the bar and is auto-approved for M56. Please go ahead and merge the CL manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

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

Comment 28 by bugdroid1@chromium.org, Jan 12 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99a040bf679e8d63f3836a7dfddbab77b95c5f22

commit 99a040bf679e8d63f3836a7dfddbab77b95c5f22
Author: lfg <lfg@chromium.org>
Date: Thu Jan 12 00:51:51 2017

Allow navigations to frames that aren't being unloaded in the unload handler.

This change lets the page navigate frames that aren't being unloaded in
the unload handler. For example, a popup may want to navigate its
opener while it's being closed.

FrameLoader now also correctly checks if the |targetFrame| can be
navigated, instead of checking if |m_frame| can be navigated.

BUG= 660496 

Review-Url: https://codereview.chromium.org/2487403002
Cr-Commit-Position: refs/heads/master@{#442353}
(cherry picked from commit 2267cfcce06c92602376754343e5f45effeee1f1)

TBR=dcheng@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2621193004
Cr-Commit-Position: refs/branch-heads/2924@{#736}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[add] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/LayoutTests/http/tests/navigation/resources/targeted-navigation-in-unload-handler-subframe.html
[add] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/LayoutTests/http/tests/navigation/targeted-navigation-in-unload-handler.html
[modify] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/Source/core/frame/History.cpp
[modify] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
[modify] https://crrev.com/99a040bf679e8d63f3836a7dfddbab77b95c5f22/third_party/WebKit/Source/core/loader/NavigationScheduler.h

Labels: TE-Verified-M56 TE-Verified-56.0.2924.67
Verified the fix on Mac 10.12.2, Win-10 and Ubuntu 14.04 using Chrome beta version #56.0.2924.67 as per the comment #0.

Observed that the original window (opener) location changed to unload.html as expected.

Hence, the fix is working as expected.

Attaching the screencast for reference

Adding the verified labels.

Thanks...!!
660496.mp4
721 KB View Download

Sign in to add a comment