New issue
Advanced search Search tips

Issue 844455 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Chrome should not allow popups during page unloading

Project Member Reported by a...@chromium.org, May 18 2018

Issue description

Cc: timothygu@chromium.org domenic@chromium.org
Had a meeting with Domenic and Timothy.

Conclusions:
- We can probably do for |window.open| what we do for dialogs, not allow them when the event loop's termination nesting level is nonzero (see https://html.spec.whatwg.org/#termination-nesting-level). This should be a pretty simple spec change.
- The structure of the WPT would be an iframe that upon unload/beforeunload/hide tries to do a window.open and put it into a variable on the parent. The parent then deletes/navigates the iframe and then can immediately check the variable to pass/fail.
- For implementation, we can reuse a lot of dialog code. For example, alert calls |ChromeClient::OpenJavaScriptAlert| which calls |ChromeClient::CanOpenModalIfDuringPageDismissal|. That's a great function that implements the check we want. It takes a parameter of |ChromeClient::DialogType| for logging purposes, so we'd want to extend that type (and rename it so it isn't entirely about "dialog"s with an enum meaning "a window.open window", and then call it right before a window.open.
Labels: Target-70
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 2

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

commit ba496ac30606b596aecf5608a1973e300f8e798b
Author: Avi Drissman <avi@chromium.org>
Date: Fri Nov 02 19:48:35 2018

Don't allow popups during page unloading times.

Precisely, this disallows them when the event loop's
termination nesting level is nonzero.

This is now part of the spec,
https://html.spec.whatwg.org/#apis-for-creating-and-navigating-browsing-contexts-by-name

> The window open steps, given a string url, a string target, and a string features, are as follows:
> 1. If the event loop's termination nesting level is nonzero, return null.

BUG= 844455 

Change-Id: I85fb003063f5ed051049c7c263391835421902ac
Reviewed-on: https://chromium-review.googlesource.com/c/1180296
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605025}
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
[add] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/WebKit/LayoutTests/external/wpt/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/no_window_open_when_term_nesting_level_nonzero.window-expected.txt
[add] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/WebKit/LayoutTests/external/wpt/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/no_window_open_when_term_nesting_level_nonzero.window.js
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/blink/renderer/core/loader/empty_clients.h
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/blink/renderer/core/page/chrome_client.cc
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/blink/renderer/core/page/chrome_client.h
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/blink/renderer/core/page/chrome_client_impl.cc
[modify] https://crrev.com/ba496ac30606b596aecf5608a1973e300f8e798b/third_party/blink/renderer/core/page/chrome_client_impl.h

Status: Fixed (was: Assigned)

Sign in to add a comment