window.open() should gate new tab/new popup based on toolbar visibility.
Previously, Chrome required that toolbar, menubar, scrollbars, status,
resizable were all set to enabled to open a window as a new tab rather
than a new popup. However, this causes developer frustration if one of
window features is accidentally omitted (as it then defaults to
disabled).
Instead, just use toolbar visibility to determine whether or not
window.open() creates a new popup or a new tab, which matches Firefox.
BUG=82522
Review-Url: https://codereview.chromium.org/2773573002
Cr-Commit-Position: refs/heads/master@{#459540}
diff --git a/third_party/WebKit/LayoutTests/fast/dom/Window/open-as-popup-vs-tab.html b/third_party/WebKit/LayoutTests/fast/dom/Window/open-as-popup-vs-tab.html
new file mode 100644
index 0000000..ba9c77a
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/dom/Window/open-as-popup-vs-tab.html
@@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+<head>
+ <meta charset="utf-8">
+ <title>window.open: popup vs tab</title>
+ <script src="../../../resources/testharness.js"></script>
+ <script src="../../../resources/testharnessreport.js"></script>
+</head>
+<body>
+ <script>
+ function openToolbarOnWindow() {
+ return window.open("", "", "toolbar=1");
+ };
+ function openToolbarOffWindow() {
+ return window.open("", "", "toolbar=0");
+ }
+ function openDefaultFeaturesWindow() {
+ return window.open();
+ }
+ test(function() {
+ var w = openToolbarOnWindow();
+ assert_equals(w.toolbar.visible, true);
+ w.close();
+ }, "window.open with toolbar=1 creates a new tab");
+ test(function() {
+ var w = openToolbarOffWindow();
+ assert_equals(w.toolbar.visible, false);
+ w.close();
+ }, "window.open with toolbar=0 creates a new popup");
+ test(function() {
+ var w = openDefaultFeaturesWindow();
+ assert_equals(w.toolbar.visible, true);
+ w.close();
+ }, "window.open defaults to creating a new tab");
+ </script>
+ <button onclick="openToolbarOnWindow()">Window Features: toolbar=1</button>
+ <button onclick="openToolbarOffWindow()">Window Features: toolbar=0</button>
+ <button onclick="openDefaultFeaturesWindow()">Window Features: default</button>
+</body>
+</html>
diff --git a/third_party/WebKit/Source/web/ChromeClientImpl.cpp b/third_party/WebKit/Source/web/ChromeClientImpl.cpp
index abb436d..513570ac 100644
--- a/third_party/WebKit/Source/web/ChromeClientImpl.cpp
+++ b/third_party/WebKit/Source/web/ChromeClientImpl.cpp
@@ -331,12 +331,19 @@
}
WebNavigationPolicy getNavigationPolicy(const WindowFeatures& features) {
- // If our default configuration was modified by a script or wasn't
- // created by a user gesture, then show as a popup. Else, let this
- // new window be opened as a toplevel window.
- bool asPopup = !features.toolBarVisible || !features.statusBarVisible ||
- !features.scrollbarsVisible || !features.menuBarVisible ||
- !features.resizable;
+ // If the window features didn't enable the toolbar, or this window wasn't
+ // created by a user gesture, show as a popup instead of a new tab.
+ //
+ // Note: this previously also checked that menubar, resizable, scrollbar, and
+ // statusbar are enabled too. When no feature string is specified, these
+ // features default to enabled (and the window opens as a new tab). However,
+ // when a feature string is specified, any *unspecified* features default to
+ // disabled, often causing the window to open as a popup instead.
+ //
+ // As specifying menubar, resizable, scrollbar, and statusbar have no effect
+ // on the UI, just ignore them and only consider whether or not the toolbar is
+ // enabled, which matches Firefox's behavior.
+ bool asPopup = !features.toolBarVisible;
NavigationPolicy policy = NavigationPolicyNewForegroundTab;
if (asPopup)
diff --git a/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp b/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp
index 183cf87..541701d 100644
--- a/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp
+++ b/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp
@@ -87,7 +87,7 @@
WebInputEvent::TimeStampForTesting);
event.button = button;
setCurrentInputEventForTest(&event);
- m_chromeClientImpl->setScrollbarsVisible(!asPopup);
+ m_chromeClientImpl->setToolbarsVisible(!asPopup);
m_chromeClientImpl->show(NavigationPolicyIgnore);
setCurrentInputEventForTest(0);
return m_result;
@@ -211,23 +211,23 @@
EXPECT_FALSE(isNavigationPolicyPopup());
}
-TEST_F(GetNavigationPolicyTest, NoStatusbarForcesPopup) {
+TEST_F(GetNavigationPolicyTest, NoStatusbarIsNotPopup) {
m_chromeClientImpl->setStatusbarVisible(false);
- EXPECT_TRUE(isNavigationPolicyPopup());
+ EXPECT_FALSE(isNavigationPolicyPopup());
m_chromeClientImpl->setStatusbarVisible(true);
EXPECT_FALSE(isNavigationPolicyPopup());
}
-TEST_F(GetNavigationPolicyTest, NoMenubarForcesPopup) {
+TEST_F(GetNavigationPolicyTest, NoMenubarIsNotPopup) {
m_chromeClientImpl->setMenubarVisible(false);
- EXPECT_TRUE(isNavigationPolicyPopup());
+ EXPECT_FALSE(isNavigationPolicyPopup());
m_chromeClientImpl->setMenubarVisible(true);
EXPECT_FALSE(isNavigationPolicyPopup());
}
-TEST_F(GetNavigationPolicyTest, NotResizableForcesPopup) {
+TEST_F(GetNavigationPolicyTest, NotResizableIsNotPopup) {
m_chromeClientImpl->setResizable(false);
- EXPECT_TRUE(isNavigationPolicyPopup());
+ EXPECT_FALSE(isNavigationPolicyPopup());
m_chromeClientImpl->setResizable(true);
EXPECT_FALSE(isNavigationPolicyPopup());
}
