From 51880be4b35e9ed23c186c9997f6f094f7def35c Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Thu, 19 Mar 2020 09:13:12 +0000 Subject: [PATCH] Bug 1507375 - Restrict the controllability of UI parts visibility with features parameter of window.open. r=smaug Make the features parameter of window.open just a condition for whether to open a popup or a new tab. Also remove dom.disable_window_open_feature.* prefs. Differential Revision: https://phabricator.services.mozilla.com/D65926 --HG-- extra : moz-landing-system : lando --- .../content/test/popups/browser_popupUI.js | 46 ++- dom/html/test/test_bug369370.html | 12 +- dom/ipc/BrowserChild.cpp | 18 +- dom/ipc/ContentChild.cpp | 36 +- dom/ipc/ContentChild.h | 4 +- dom/ipc/ContentParent.cpp | 40 ++- dom/ipc/ContentParent.h | 17 +- dom/ipc/PContent.ipdl | 6 +- dom/tests/mochitest/bugs/test_window_bar.html | 129 +++++--- ...test_transformed_scrolling_repaints_3.html | 2 +- ...ansformed_scrolling_repaints_3_window.html | 2 +- .../windowcreator/nsIWindowProvider.idl | 10 +- .../windowwatcher/nsWindowWatcher.cpp | 171 ++++++---- .../windowwatcher/nsWindowWatcher.h | 15 +- .../components/windowwatcher/test/browser.ini | 5 + .../browser_new_content_window_chromeflags.js | 88 +---- .../test/browser_non_popup_from_popup.js | 53 +++ .../test/browser_popup_condition.js | 308 ++++++++++++++++++ toolkit/components/windowwatcher/test/head.js | 14 + xpfe/appshell/nsContentTreeOwner.cpp | 9 +- 20 files changed, 700 insertions(+), 285 deletions(-) create mode 100644 toolkit/components/windowwatcher/test/browser_non_popup_from_popup.js create mode 100644 toolkit/components/windowwatcher/test/browser_popup_condition.js create mode 100644 toolkit/components/windowwatcher/test/head.js diff --git a/browser/base/content/test/popups/browser_popupUI.js b/browser/base/content/test/popups/browser_popupUI.js index a393902838ee..b953a0ef549b 100644 --- a/browser/base/content/test/popups/browser_popupUI.js +++ b/browser/base/content/test/popups/browser_popupUI.js @@ -61,25 +61,34 @@ add_task(async function titlebar_buttons_visibility() { const BUTTONS_MAY_VISIBLE = true; const BUTTONS_NEVER_VISIBLE = false; - const drawInTitlebarValues = { - true: BUTTONS_MAY_VISIBLE, - false: BUTTONS_NEVER_VISIBLE, - }; - const windowFeaturesValues = { - "width=300,height=100": BUTTONS_NEVER_VISIBLE, - toolbar: BUTTONS_MAY_VISIBLE, - menubar: BUTTONS_NEVER_VISIBLE, - "menubar,toolbar": BUTTONS_MAY_VISIBLE, - }; + // Always open a new window. + // With default behavior, it opens a new tab, that doesn't affect button + // visibility at all. + Services.prefs.setIntPref("browser.link.open_newwindow", 2); + + const drawInTitlebarValues = [ + [true, BUTTONS_MAY_VISIBLE], + [false, BUTTONS_NEVER_VISIBLE], + ]; + const windowFeaturesValues = [ + // Opens a popup + ["width=300,height=100", BUTTONS_NEVER_VISIBLE], + ["toolbar", BUTTONS_NEVER_VISIBLE], + ["menubar", BUTTONS_NEVER_VISIBLE], + ["menubar,toolbar", BUTTONS_NEVER_VISIBLE], + + // Opens a new window + ["", BUTTONS_MAY_VISIBLE], + ]; const menuBarShownValues = [true, false]; - for (const drawInTitlebar of Object.keys(drawInTitlebarValues)) { - Services.prefs.setBoolPref( - "browser.tabs.drawInTitlebar", - drawInTitlebar == "true" - ); + for (const [drawInTitlebar, drawInTitlebarButtons] of drawInTitlebarValues) { + Services.prefs.setBoolPref("browser.tabs.drawInTitlebar", drawInTitlebar); - for (const windowFeatures of Object.keys(windowFeaturesValues)) { + for (const [ + windowFeatures, + windowFeaturesButtons, + ] of windowFeaturesValues) { for (const menuBarShown of menuBarShownValues) { CustomizableUI.setToolbarVisibility("toolbar-menubar", menuBarShown); @@ -109,8 +118,8 @@ add_task(async function titlebar_buttons_visibility() { const params = `drawInTitlebar=${drawInTitlebar}, windowFeatures=${windowFeatures}, menuBarShown=${menuBarShown}`; if ( - drawInTitlebarValues[drawInTitlebar] == BUTTONS_MAY_VISIBLE && - windowFeaturesValues[windowFeatures] == BUTTONS_MAY_VISIBLE + drawInTitlebarButtons == BUTTONS_MAY_VISIBLE && + windowFeaturesButtons == BUTTONS_MAY_VISIBLE ) { ok( buttonsInMenubarShown || buttonsInTabbarShown, @@ -137,4 +146,5 @@ add_task(async function titlebar_buttons_visibility() { CustomizableUI.setToolbarVisibility("toolbar-menubar", false); Services.prefs.clearUserPref("browser.tabs.drawInTitlebar"); + Services.prefs.clearUserPref("browser.link.open_newwindow"); }); diff --git a/dom/html/test/test_bug369370.html b/dom/html/test/test_bug369370.html index 9d3c5d305b95..ee87c1d2de6a 100644 --- a/dom/html/test/test_bug369370.html +++ b/dom/html/test/test_bug369370.html @@ -74,8 +74,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=369370 is(img.width, 800, "image width"); is(img.height, 600, "image height"); - is(kidDoc.body.scrollLeft, 400, "Checking scrollLeft"); - is(kidDoc.body.scrollTop, 300, "Checking scrollTop"); + is(kidDoc.body.scrollLeft, + kidDoc.body.scrollLeftMax, "Checking scrollLeft"); + is(kidDoc.body.scrollTop, + kidDoc.body.scrollTopMax, "Checking scrollTop"); // ========== test 4 ========== // Click there again to zoom out @@ -106,7 +108,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=369370 // Give the image document time to respond SimpleTest.executeSoon(function() { is(img.height, 600, "image height"); - is(img.getBoundingClientRect().top, 25, "Image is vertically centered"); + var bodyHeight = kidDoc.body.scrollHeight; + var imgRect = img.getBoundingClientRect(); + is(imgRect.top, bodyHeight - imgRect.bottom, "Image is vertically centered"); test7(); }); }, {once: true}); @@ -139,7 +143,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=369370 SimpleTest.waitForExplicitFinish(); SpecialPowers.pushPrefEnv({"set":[["browser.enable_automatic_image_resizing", true]]}, function() { - kidWin = window.open("bug369370-popup.png", "bug369370", "width=400,height=300,scrollbars=no"); + kidWin = window.open("bug369370-popup.png", "bug369370", "width=400,height=300"); // will init onload ok(kidWin, "opened child window"); kidWin.onload = childLoaded; diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 9af084001642..d43a040f2083 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -878,10 +878,10 @@ BrowserChild::GetInterface(const nsIID& aIID, void** aSink) { NS_IMETHODIMP BrowserChild::ProvideWindow(mozIDOMWindowProxy* aParent, uint32_t aChromeFlags, - bool aCalledFromJS, bool aPositionSpecified, - bool aSizeSpecified, nsIURI* aURI, - const nsAString& aName, const nsACString& aFeatures, - bool aForceNoOpener, bool aForceNoReferrer, + bool aCalledFromJS, bool aWidthSpecified, + nsIURI* aURI, const nsAString& aName, + const nsACString& aFeatures, bool aForceNoOpener, + bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew, BrowsingContext** aReturn) { *aReturn = nullptr; @@ -899,7 +899,7 @@ BrowserChild::ProvideWindow(mozIDOMWindowProxy* aParent, uint32_t aChromeFlags, if (!iframeMoz) { int32_t openLocation = nsWindowWatcher::GetWindowOpenLocation( nsPIDOMWindowOuter::From(aParent), aChromeFlags, aCalledFromJS, - aPositionSpecified, aSizeSpecified); + aWidthSpecified); // If it turns out we're opening in the current browser, just hand over the // current browser's docshell. @@ -921,10 +921,10 @@ BrowserChild::ProvideWindow(mozIDOMWindowProxy* aParent, uint32_t aChromeFlags, // open window call was canceled. It's important that we pass this error // code back to our caller. ContentChild* cc = ContentChild::GetSingleton(); - return cc->ProvideWindowCommon( - this, aParent, iframeMoz, aChromeFlags, aCalledFromJS, aPositionSpecified, - aSizeSpecified, aURI, aName, aFeatures, aForceNoOpener, aForceNoReferrer, - aLoadState, aWindowIsNew, aReturn); + return cc->ProvideWindowCommon(this, aParent, iframeMoz, aChromeFlags, + aCalledFromJS, aWidthSpecified, aURI, aName, + aFeatures, aForceNoOpener, aForceNoReferrer, + aLoadState, aWindowIsNew, aReturn); } void BrowserChild::DestroyWindow() { diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index e4e55143ac28..4330f7edb690 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -775,16 +775,16 @@ void ContentChild::SetProcessName(const nsAString& aName) { NS_IMETHODIMP ContentChild::ProvideWindow(mozIDOMWindowProxy* aParent, uint32_t aChromeFlags, - bool aCalledFromJS, bool aPositionSpecified, - bool aSizeSpecified, nsIURI* aURI, - const nsAString& aName, const nsACString& aFeatures, - bool aForceNoOpener, bool aForceNoReferrer, + bool aCalledFromJS, bool aWidthSpecified, + nsIURI* aURI, const nsAString& aName, + const nsACString& aFeatures, bool aForceNoOpener, + bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew, BrowsingContext** aReturn) { - return ProvideWindowCommon( - nullptr, aParent, false, aChromeFlags, aCalledFromJS, aPositionSpecified, - aSizeSpecified, aURI, aName, aFeatures, aForceNoOpener, aForceNoReferrer, - aLoadState, aWindowIsNew, aReturn); + return ProvideWindowCommon(nullptr, aParent, false, aChromeFlags, + aCalledFromJS, aWidthSpecified, aURI, aName, + aFeatures, aForceNoOpener, aForceNoReferrer, + aLoadState, aWindowIsNew, aReturn); } static nsresult GetCreateWindowParams(mozIDOMWindowProxy* aParent, @@ -863,11 +863,10 @@ static nsresult GetCreateWindowParams(mozIDOMWindowProxy* aParent, nsresult ContentChild::ProvideWindowCommon( BrowserChild* aTabOpener, mozIDOMWindowProxy* aParent, bool aIframeMoz, - uint32_t aChromeFlags, bool aCalledFromJS, bool aPositionSpecified, - bool aSizeSpecified, nsIURI* aURI, const nsAString& aName, - const nsACString& aFeatures, bool aForceNoOpener, bool aForceNoReferrer, - nsDocShellLoadState* aLoadState, bool* aWindowIsNew, - BrowsingContext** aReturn) { + uint32_t aChromeFlags, bool aCalledFromJS, bool aWidthSpecified, + nsIURI* aURI, const nsAString& aName, const nsACString& aFeatures, + bool aForceNoOpener, bool aForceNoReferrer, nsDocShellLoadState* aLoadState, + bool* aWindowIsNew, BrowsingContext** aReturn) { *aReturn = nullptr; nsAutoPtr ipcContext; @@ -944,9 +943,8 @@ nsresult ContentChild::ProvideWindowCommon( MOZ_DIAGNOSTIC_ASSERT(!nsContentUtils::IsSpecialName(name)); Unused << SendCreateWindowInDifferentProcess( - aTabOpener, aChromeFlags, aCalledFromJS, aPositionSpecified, - aSizeSpecified, uriToLoad, features, fullZoom, name, - triggeringPrincipal, csp, referrerInfo); + aTabOpener, aChromeFlags, aCalledFromJS, aWidthSpecified, uriToLoad, + features, fullZoom, name, triggeringPrincipal, csp, referrerInfo); // We return NS_ERROR_ABORT, so that the caller knows that we've abandoned // the window open as far as it is concerned. @@ -1199,9 +1197,9 @@ nsresult ContentChild::ProvideWindowCommon( } SendCreateWindow(aTabOpener, newChild, aChromeFlags, aCalledFromJS, - aPositionSpecified, aSizeSpecified, uriToLoad, features, - fullZoom, Principal(triggeringPrincipal), csp, - referrerInfo, std::move(resolve), std::move(reject)); + aWidthSpecified, uriToLoad, features, fullZoom, + Principal(triggeringPrincipal), csp, referrerInfo, + std::move(resolve), std::move(reject)); } // ======================= diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 6a37a02e0ac9..c59d04cd650b 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -112,8 +112,8 @@ class ContentChild final nsresult ProvideWindowCommon(BrowserChild* aTabOpener, mozIDOMWindowProxy* aParent, bool aIframeMoz, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, bool aSizeSpecified, - nsIURI* aURI, const nsAString& aName, + bool aWidthSpecified, nsIURI* aURI, + const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener, bool aForceNoReferrer, nsDocShellLoadState* aLoadState, diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index ada3e5234dd0..2c3fda27c080 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -4619,13 +4619,13 @@ bool ContentParent::DeallocPWebBrowserPersistDocumentParent( mozilla::ipc::IPCResult ContentParent::CommonCreateWindow( PBrowserParent* aThisTab, bool aSetOpener, const uint32_t& aChromeFlags, - const bool& aCalledFromJS, const bool& aPositionSpecified, - const bool& aSizeSpecified, nsIURI* aURIToLoad, const nsCString& aFeatures, - const float& aFullZoom, uint64_t aNextRemoteTabId, const nsString& aName, - nsresult& aResult, nsCOMPtr& aNewRemoteTab, - bool* aWindowIsNew, int32_t& aOpenLocation, - nsIPrincipal* aTriggeringPrincipal, nsIReferrerInfo* aReferrerInfo, - bool aLoadURI, nsIContentSecurityPolicy* aCsp) + const bool& aCalledFromJS, const bool& aWidthSpecified, nsIURI* aURIToLoad, + const nsCString& aFeatures, const float& aFullZoom, + uint64_t aNextRemoteTabId, const nsString& aName, nsresult& aResult, + nsCOMPtr& aNewRemoteTab, bool* aWindowIsNew, + int32_t& aOpenLocation, nsIPrincipal* aTriggeringPrincipal, + nsIReferrerInfo* aReferrerInfo, bool aLoadURI, + nsIContentSecurityPolicy* aCsp) { // The content process should never be in charge of computing whether or @@ -4699,8 +4699,7 @@ mozilla::ipc::IPCResult ContentParent::CommonCreateWindow( } aOpenLocation = nsWindowWatcher::GetWindowOpenLocation( - outerWin, aChromeFlags, aCalledFromJS, aPositionSpecified, - aSizeSpecified); + outerWin, aChromeFlags, aCalledFromJS, aWidthSpecified); MOZ_ASSERT(aOpenLocation == nsIBrowserDOMWindow::OPEN_NEWTAB || aOpenLocation == nsIBrowserDOMWindow::OPEN_NEWWINDOW); @@ -4841,11 +4840,10 @@ mozilla::ipc::IPCResult ContentParent::CommonCreateWindow( mozilla::ipc::IPCResult ContentParent::RecvCreateWindow( PBrowserParent* aThisTab, PBrowserParent* aNewTab, const uint32_t& aChromeFlags, const bool& aCalledFromJS, - const bool& aPositionSpecified, const bool& aSizeSpecified, - const Maybe& aURIToLoad, const nsCString& aFeatures, - const float& aFullZoom, const IPC::Principal& aTriggeringPrincipal, - nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo, - CreateWindowResolver&& aResolve) { + const bool& aWidthSpecified, const Maybe& aURIToLoad, + const nsCString& aFeatures, const float& aFullZoom, + const IPC::Principal& aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp, + nsIReferrerInfo* aReferrerInfo, CreateWindowResolver&& aResolve) { nsresult rv = NS_OK; CreatedWindowInfo cwi; @@ -4884,9 +4882,9 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindow( int32_t openLocation = nsIBrowserDOMWindow::OPEN_NEWWINDOW; mozilla::ipc::IPCResult ipcResult = CommonCreateWindow( aThisTab, /* aSetOpener = */ true, aChromeFlags, aCalledFromJS, - aPositionSpecified, aSizeSpecified, uriToLoad, aFeatures, aFullZoom, - nextRemoteTabId, VoidString(), rv, newRemoteTab, &cwi.windowOpened(), - openLocation, aTriggeringPrincipal, aReferrerInfo, + aWidthSpecified, uriToLoad, aFeatures, aFullZoom, nextRemoteTabId, + VoidString(), rv, newRemoteTab, &cwi.windowOpened(), openLocation, + aTriggeringPrincipal, aReferrerInfo, /* aLoadUri = */ false, aCsp); if (!ipcResult) { return ipcResult; @@ -4918,9 +4916,9 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindow( mozilla::ipc::IPCResult ContentParent::RecvCreateWindowInDifferentProcess( PBrowserParent* aThisTab, const uint32_t& aChromeFlags, - const bool& aCalledFromJS, const bool& aPositionSpecified, - const bool& aSizeSpecified, const Maybe& aURIToLoad, - const nsCString& aFeatures, const float& aFullZoom, const nsString& aName, + const bool& aCalledFromJS, const bool& aWidthSpecified, + const Maybe& aURIToLoad, const nsCString& aFeatures, + const float& aFullZoom, const nsString& aName, nsIPrincipal* aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo) { MOZ_DIAGNOSTIC_ASSERT(!nsContentUtils::IsSpecialName(aName)); @@ -4958,7 +4956,7 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindowInDifferentProcess( nsresult rv; mozilla::ipc::IPCResult ipcResult = CommonCreateWindow( aThisTab, /* aSetOpener = */ false, aChromeFlags, aCalledFromJS, - aPositionSpecified, aSizeSpecified, uriToLoad, aFeatures, aFullZoom, + aWidthSpecified, uriToLoad, aFeatures, aFullZoom, /* aNextRemoteTabId = */ 0, aName, rv, newRemoteTab, &windowIsNew, openLocation, aTriggeringPrincipal, aReferrerInfo, /* aLoadUri = */ true, aCsp); diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 4e27016b2e24..34859208babd 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -521,17 +521,17 @@ class ContentParent final mozilla::ipc::IPCResult RecvCreateWindow( PBrowserParent* aThisBrowserParent, PBrowserParent* aNewTab, const uint32_t& aChromeFlags, const bool& aCalledFromJS, - const bool& aPositionSpecified, const bool& aSizeSpecified, - const Maybe& aURIToLoad, const nsCString& aFeatures, - const float& aFullZoom, const IPC::Principal& aTriggeringPrincipal, + const bool& aWidthSpecified, const Maybe& aURIToLoad, + const nsCString& aFeatures, const float& aFullZoom, + const IPC::Principal& aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo, CreateWindowResolver&& aResolve); mozilla::ipc::IPCResult RecvCreateWindowInDifferentProcess( PBrowserParent* aThisTab, const uint32_t& aChromeFlags, - const bool& aCalledFromJS, const bool& aPositionSpecified, - const bool& aSizeSpecified, const Maybe& aURIToLoad, - const nsCString& aFeatures, const float& aFullZoom, const nsString& aName, + const bool& aCalledFromJS, const bool& aWidthSpecified, + const Maybe& aURIToLoad, const nsCString& aFeatures, + const float& aFullZoom, const nsString& aName, nsIPrincipal* aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo); @@ -727,9 +727,8 @@ class ContentParent final // compatibility with GeckoView. mozilla::ipc::IPCResult CommonCreateWindow( PBrowserParent* aThisTab, bool aSetOpener, const uint32_t& aChromeFlags, - const bool& aCalledFromJS, const bool& aPositionSpecified, - const bool& aSizeSpecified, nsIURI* aURIToLoad, - const nsCString& aFeatures, const float& aFullZoom, + const bool& aCalledFromJS, const bool& aWidthSpecified, + nsIURI* aURIToLoad, const nsCString& aFeatures, const float& aFullZoom, uint64_t aNextRemoteTabId, const nsString& aName, nsresult& aResult, nsCOMPtr& aNewRemoteTab, bool* aWindowIsNew, int32_t& aOpenLocation, nsIPrincipal* aTriggeringPrincipal, diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 997d8b45df87..ad804ed0b74b 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1354,8 +1354,7 @@ parent: PBrowser aNewTab, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, - bool aSizeSpecified, + bool aWidthSpecified, URIParams? aURIToLoad, nsCString aFeatures, float aFullZoom, @@ -1368,8 +1367,7 @@ parent: PBrowser aThisTab, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, - bool aSizeSpecified, + bool aWidthSpecified, URIParams? aURIToLoad, nsCString aFeatures, float aFullZoom, diff --git a/dom/tests/mochitest/bugs/test_window_bar.html b/dom/tests/mochitest/bugs/test_window_bar.html index 22e7a1e8bcb8..195a24e6e744 100644 --- a/dom/tests/mochitest/bugs/test_window_bar.html +++ b/dom/tests/mochitest/bugs/test_window_bar.html @@ -28,43 +28,106 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=642338 */ +// Popup is opened if one of the following condtitions is met: +// * both location and toolbar are false +// * menubar is false +// * resizable is false +// * scrollbars is false +// * status is false +// * width is specified +var featuresList = [ + { + type: 'non-popup', + target: 'all-bars-toolbar', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=yes', + }, + { + type: 'non-popup', + target: 'all-bars-location', + features: 'location=yes,menubar=yes,status=yes,scrollbars=yes', + }, + { + type: 'popup', + target: 'no-menubar', + features: 'toolbar=yes,menubar=no,status=yes,scrollbars=yes', + }, + { + type: 'popup', + target: 'no-toolbar', + features: 'toolbar=no,menubar=yes,status=yes,scrollbars=yes', + }, + { + type: 'popup', + target: 'no-status', + features: 'toolbar=yes,menubar=yes,status=no,scrollbars=yes', + }, + { + type: 'popup', + target: 'no-scrollbars', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=no', + }, + { + type: 'popup', + target: 'no-bars', + features: 'toolbar=no,menubar=no,status=no,scrollbars=no', + }, + { + type: 'popup', + target: 'no-resizable', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=yes' + + ',resizable=no', + }, + { + type: 'popup', + target: 'width-sized', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=yes' + + ',width=500', + }, + { + type: 'non-popup', + target: 'height-sized', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=yes' + + ',height=500', + }, + { + type: 'popup', + target: 'sized', + features: 'toolbar=yes,menubar=yes,status=yes,scrollbars=yes' + + ',width=500,height=500', + }, +]; + var numWindows = 0; /* Called when our popup loads. */ function testWindow(w) { - - // If dom.disable_window_open_feature.X is true, then we can't disable - // feature X when we call window.open from content. So to check that our popup - // has the right bars shown, we need to check that it obeys first the pref - // and then the arguments to window.open, if applicable. - - function checkFeature(feature, prefname) { - if (prefname === undefined) - prefname = feature; - - if (SpecialPowers.getBoolPref('dom.disable_window_open_feature.' + prefname)) { - is(w[feature].visible, true, feature + ' should always be true.'); - } - else { - // w.location.search == '?true' if we expect the bars to be on, and - // '?false' otherwise. By default, no bars are enabled, so '?default' - // can be handled the same way as '?false'. - var enabled = w.location.search == '?true'; - is(w[feature].visible, enabled, feature + ' should follow window.open settings.'); + function checkFeature(feature) { + if (w.location.search.startsWith('?popup')) { + is(w[feature].visible, false, `${feature} should be hidden for popup (${w.name}).`); + } else { + is(w[feature].visible, true, `${feature} should be visible for non-popup (${w.name}).`); } } + // If popup is opened, the following properties become false: + // * menubar.visible + // * toolbar.visible + // * personalbar.visible + // * status.visible checkFeature('menubar'); checkFeature('toolbar'); checkFeature('personalbar'); - checkFeature('statusbar', 'status'); - checkFeature('locationbar', 'location'); + + // The following aren't affected by feature. + is(w.scrollbars.visible, true, `scrollbars should always be visible (${w.name}).`); + is(w.statusbar.visible, true, `statusbar should always be visible (${w.name}).`); + is(w.locationbar.visible, true, `locationbar should always be visible (${w.name}).`); w.close(); numWindows++; - if (numWindows == 3) { + if (numWindows == featuresList.length) { // We're done! SimpleTest.finish(); } @@ -72,24 +135,10 @@ function testWindow(w) } SimpleTest.waitForExplicitFinish(); - -// These will call back into testWindow when they open. - -var allBarsWindow = - window.open('file_window_bar.html?true', 'all-bars', - 'menubar=yes,toolbar=yes,location=yes,' + - 'personalbar=yes,status=yes,scrollbars=yes', - true); - -var noBarsWindow = - window.open('file_window_bar.html?false', 'no-bars', - 'menubar=no,toolbar=no,location=no,' + - 'personalbar=no,status=no,scrollbars=no', - false); - -var defaultWindow = - window.open('file_window_bar.html?default', 'default-bars', - 'width=500,height=500', false); +for (var item of featuresList) { + // This will call back into testWindow when they open. + window.open(`file_window_bar.html?${item.type}`, item.target, item.features); +} diff --git a/layout/base/tests/test_transformed_scrolling_repaints_3.html b/layout/base/tests/test_transformed_scrolling_repaints_3.html index f1823c52f8d1..eb9ad9ba937d 100644 --- a/layout/base/tests/test_transformed_scrolling_repaints_3.html +++ b/layout/base/tests/test_transformed_scrolling_repaints_3.html @@ -15,7 +15,7 @@ SpecialPowers.pushPrefEnv( {"set": [["layers.single-tile.enabled", false]]}, function() { window.open("transformed_scrolling_repaints_3_window.html", "transformed_scrolling_repaints_3", - "chrome,width=300,height=400,scrollbars=no"); + "chrome,width=350,height=450"); } ); diff --git a/layout/base/tests/transformed_scrolling_repaints_3_window.html b/layout/base/tests/transformed_scrolling_repaints_3_window.html index 1b345a25d082..ae6a05294d5a 100644 --- a/layout/base/tests/transformed_scrolling_repaints_3_window.html +++ b/layout/base/tests/transformed_scrolling_repaints_3_window.html @@ -1,5 +1,5 @@ - + Test that scaled elements with scrolled contents don't repaint unnecessarily when we scroll inside them diff --git a/toolkit/components/windowcreator/nsIWindowProvider.idl b/toolkit/components/windowcreator/nsIWindowProvider.idl index 20e15d61bae3..c3fc3409459b 100644 --- a/toolkit/components/windowcreator/nsIWindowProvider.idl +++ b/toolkit/components/windowcreator/nsIWindowProvider.idl @@ -49,11 +49,8 @@ interface nsIWindowProvider : nsISupports * window if this provider returns null. See nsIWebBrowserChrome for * the possible values of this field. * - * @param aPositionSpecified Whether the attempt to create a window is trying - * to specify a position for the new window. - * - * @param aSizeSpecified Whether the attempt to create a window is trying to - * specify a size for the new window. + * @param aWidthSpecified Whether the attempt to create a window is trying + * to specify the width for the new window. * * @param aURI The URI to be loaded in the new window (may be NULL). The * nsIWindowProvider implementation must not load this URI into the @@ -101,8 +98,7 @@ interface nsIWindowProvider : nsISupports BrowsingContext provideWindow(in mozIDOMWindowProxy aParent, in unsigned long aChromeFlags, in boolean aCalledFromJS, - in boolean aPositionSpecified, - in boolean aSizeSpecified, + in boolean aWidthSpecified, in nsIURI aURI, in AString aName, in AUTF8String aFeatures, diff --git a/toolkit/components/windowwatcher/nsWindowWatcher.cpp b/toolkit/components/windowwatcher/nsWindowWatcher.cpp index 0c19dedf19ee..74f272045c70 100644 --- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp +++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp @@ -343,9 +343,14 @@ struct SizeSpec { bool PositionSpecified() const { return mLeftSpecified || mTopSpecified; } - bool SizeSpecified() const { - return mOuterWidthSpecified || mOuterHeightSpecified || - mInnerWidthSpecified || mInnerHeightSpecified; + bool SizeSpecified() const { return WidthSpecified() || HeightSpecified(); } + + bool WidthSpecified() const { + return mOuterWidthSpecified || mInnerWidthSpecified; + } + + bool HeightSpecified() const { + return mOuterHeightSpecified || mInnerHeightSpecified; } }; @@ -517,7 +522,10 @@ nsWindowWatcher::OpenWindowWithRemoteTab( return NS_ERROR_UNEXPECTED; } - uint32_t chromeFlags = CalculateChromeFlagsForChild(aFeatures); + SizeSpec sizeSpec; + CalcSizeSpec(aFeatures, sizeSpec); + + uint32_t chromeFlags = CalculateChromeFlagsForChild(aFeatures, sizeSpec); // A content process has asked for a new window, which implies // that the new window will need to be remote. @@ -564,8 +572,6 @@ nsWindowWatcher::OpenWindowWithRemoteTab( MaybeDisablePersistence(aFeatures, chromeTreeOwner); - SizeSpec sizeSpec; - CalcSizeSpec(aFeatures, sizeSpec); SizeOpenedWindow(chromeTreeOwner, parentWindowOuter, false, sizeSpec, Some(aOpenerFullZoom)); @@ -685,16 +691,19 @@ nsresult nsWindowWatcher::OpenWindowInternal( bool isCallerChrome = nsContentUtils::LegacyIsCallerChromeOrNativeCode(); + SizeSpec sizeSpec; + CalcSizeSpec(features, sizeSpec); + // Make sure we calculate the chromeFlags *before* we push the // callee context onto the context stack so that // the calculation sees the actual caller when doing its // security checks. if (isCallerChrome && XRE_IsParentProcess()) { - chromeFlags = CalculateChromeFlagsForParent(aParent, features, aDialog, - uriToLoadIsChrome, + chromeFlags = CalculateChromeFlagsForParent(aParent, features, sizeSpec, + aDialog, uriToLoadIsChrome, hasChromeParent, aCalledFromJS); } else { - chromeFlags = CalculateChromeFlagsForChild(features); + chromeFlags = CalculateChromeFlagsForChild(features, sizeSpec); if (aDialog) { MOZ_ASSERT(XRE_IsParentProcess()); @@ -716,9 +725,6 @@ nsresult nsWindowWatcher::OpenWindowInternal( } } - SizeSpec sizeSpec; - CalcSizeSpec(features, sizeSpec); - // XXXbz Why is an AutoJSAPI good enough here? Wouldn't AutoEntryScript (so // we affect the entry global) make more sense? Or do we just want to affect // GetSubjectPrincipal()? @@ -803,9 +809,9 @@ nsresult nsWindowWatcher::OpenWindowInternal( if (provider) { rv = provider->ProvideWindow( - aParent, chromeFlags, aCalledFromJS, sizeSpec.PositionSpecified(), - sizeSpec.SizeSpecified(), uriToLoad, name, features, aForceNoOpener, - aForceNoReferrer, aLoadState, &windowIsNew, getter_AddRefs(newBC)); + aParent, chromeFlags, aCalledFromJS, sizeSpec.WidthSpecified(), + uriToLoad, name, features, aForceNoOpener, aForceNoReferrer, + aLoadState, &windowIsNew, getter_AddRefs(newBC)); if (NS_SUCCEEDED(rv) && newBC) { nsCOMPtr newDocShell = newBC->GetDocShell(); @@ -1646,37 +1652,17 @@ nsresult nsWindowWatcher::URIfromURL(const char* aURL, return NS_NewURI(aURI, aURL, baseURI); } -#define NS_CALCULATE_CHROME_FLAG_FOR(feature, flag) \ - prefBranch->GetBoolPref(feature, &forceEnable); \ - if (forceEnable && !aDialog && !aHasChromeParent && !aChromeURL) { \ - chromeFlags |= flag; \ - } else { \ - chromeFlags |= \ - WinHasOption(aFeatures, feature, 0, &presenceFlag) ? flag : 0; \ - } - +#define NS_CALCULATE_CHROME_FLAG_FOR(feature, flag) \ + chromeFlags |= \ + WinHasOption(aFeatures, (feature), 0, &presenceFlag) ? (flag) : 0; // static uint32_t nsWindowWatcher::CalculateChromeFlagsHelper( - uint32_t aInitialFlags, const nsACString& aFeatures, bool& presenceFlag, - bool aDialog, bool aHasChromeParent, bool aChromeURL) { + uint32_t aInitialFlags, const nsACString& aFeatures, + const SizeSpec& aSizeSpec, bool& presenceFlag, bool aHasChromeParent) { uint32_t chromeFlags = aInitialFlags; - nsresult rv; - nsCOMPtr prefBranch; - nsCOMPtr prefs = - do_GetService(NS_PREFSERVICE_CONTRACTID, &rv); - - NS_ENSURE_SUCCESS(rv, nsIWebBrowserChrome::CHROME_DEFAULT); - - rv = prefs->GetBranch("dom.disable_window_open_feature.", - getter_AddRefs(prefBranch)); - - NS_ENSURE_SUCCESS(rv, nsIWebBrowserChrome::CHROME_DEFAULT); - - // NS_CALCULATE_CHROME_FLAG_FOR requires aFeatures, forceEnable, aDialog - // aHasChromeParent, aChromeURL, presenceFlag and chromeFlags to be in - // scope. - bool forceEnable = false; + // NS_CALCULATE_CHROME_FLAG_FOR requires aFeatures, presenceFlag, and + // chromeFlags to be in scope. NS_CALCULATE_CHROME_FLAG_FOR("titlebar", nsIWebBrowserChrome::CHROME_TITLEBAR); @@ -1702,7 +1688,33 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsHelper( } presenceFlag = presenceFlag || scrollbarsPresent; - return chromeFlags; + if (aHasChromeParent) { + return chromeFlags; + } + + // Web content isn't allowed to control UI visibility separately, but only + // whether to open a popup or not. + // + // The above code is still necessary to calculate `presenceFlag`. + // (`ShouldOpenPopup` early returns and doesn't check all feature) + + if (ShouldOpenPopup(aFeatures, aSizeSpec)) { + // Flags for opening a popup, that doesn't have the following: + // * nsIWebBrowserChrome::CHROME_TOOLBAR + // * nsIWebBrowserChrome::CHROME_PERSONAL_TOOLBAR + // * nsIWebBrowserChrome::CHROME_MENUBAR + return aInitialFlags | nsIWebBrowserChrome::CHROME_TITLEBAR | + nsIWebBrowserChrome::CHROME_WINDOW_CLOSE | + nsIWebBrowserChrome::CHROME_LOCATIONBAR | + nsIWebBrowserChrome::CHROME_STATUSBAR | + nsIWebBrowserChrome::CHROME_WINDOW_RESIZE | + nsIWebBrowserChrome::CHROME_WINDOW_MIN | + nsIWebBrowserChrome::CHROME_SCROLLBARS; + } + + // Otherwise open the current/new tab in the current/new window + // (depends on browser.link.open_newwindow). + return aInitialFlags | nsIWebBrowserChrome::CHROME_ALL; } // static @@ -1729,23 +1741,68 @@ uint32_t nsWindowWatcher::EnsureFlagsSafeForContent(uint32_t aChromeFlags, return aChromeFlags; } +// static +bool nsWindowWatcher::ShouldOpenPopup(const nsACString& aFeatures, + const SizeSpec& aSizeSpec) { + if (aFeatures.IsVoid()) { + return false; + } + + // Follow Google Chrome's behavior that opens a popup depending on + // the following features. + bool unused; + if (!WinHasOption(aFeatures, "location", 0, &unused) && + !WinHasOption(aFeatures, "toolbar", 0, &unused)) { + return true; + } + + if (!WinHasOption(aFeatures, "menubar", 0, &unused)) { + return true; + } + + // `resizable` defaults to true. + // Should open popup only when explicitly specified to 0. + bool resizablePresent = false; + if (!WinHasOption(aFeatures, "resizable", 0, &resizablePresent) && + resizablePresent) { + return true; + } + + if (!WinHasOption(aFeatures, "scrollbars", 0, &unused)) { + return true; + } + + if (!WinHasOption(aFeatures, "status", 0, &unused)) { + return true; + } + + // Follow Safari's behavior that opens a popup when width is specified. + if (aSizeSpec.WidthSpecified()) { + return true; + } + + return false; +} + /** * Calculate the chrome bitmask from a string list of features requested - * from a child process. Feature strings that are restricted to the parent - * process are ignored here. + * from a child process. The feature string can only control whether to open a + * new tab or a new popup. * @param aFeatures a string containing a list of named features + * @param aSizeSpec the result of CalcSizeSpec * @return the chrome bitmask */ // static uint32_t nsWindowWatcher::CalculateChromeFlagsForChild( - const nsACString& aFeatures) { + const nsACString& aFeatures, const SizeSpec& aSizeSpec) { if (aFeatures.IsVoid()) { return nsIWebBrowserChrome::CHROME_ALL; } bool presenceFlag = false; - uint32_t chromeFlags = CalculateChromeFlagsHelper( - nsIWebBrowserChrome::CHROME_WINDOW_BORDERS, aFeatures, presenceFlag); + uint32_t chromeFlags = + CalculateChromeFlagsHelper(nsIWebBrowserChrome::CHROME_WINDOW_BORDERS, + aFeatures, aSizeSpec, presenceFlag); return EnsureFlagsSafeForContent(chromeFlags); } @@ -1763,8 +1820,9 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsForChild( */ // static uint32_t nsWindowWatcher::CalculateChromeFlagsForParent( - mozIDOMWindowProxy* aParent, const nsACString& aFeatures, bool aDialog, - bool aChromeURL, bool aHasChromeParent, bool aCalledFromJS) { + mozIDOMWindowProxy* aParent, const nsACString& aFeatures, + const SizeSpec& aSizeSpec, bool aDialog, bool aChromeURL, + bool aHasChromeParent, bool aCalledFromJS) { MOZ_ASSERT(XRE_IsParentProcess()); MOZ_ASSERT(nsContentUtils::LegacyIsCallerChromeOrNativeCode()); @@ -1796,9 +1854,8 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsForParent( } /* Next, allow explicitly named options to override the initial settings */ - chromeFlags = - CalculateChromeFlagsHelper(chromeFlags, aFeatures, presenceFlag, aDialog, - aHasChromeParent, aChromeURL); + chromeFlags = CalculateChromeFlagsHelper(chromeFlags, aFeatures, aSizeSpec, + presenceFlag, aHasChromeParent); // Determine whether the window is a private browsing window chromeFlags |= WinHasOption(aFeatures, "private", 0, &presenceFlag) @@ -2402,8 +2459,7 @@ void nsWindowWatcher::GetWindowTreeOwner(nsPIDOMWindowOuter* aWindow, int32_t nsWindowWatcher::GetWindowOpenLocation(nsPIDOMWindowOuter* aParent, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, - bool aSizeSpecified) { + bool aWidthSpecified) { bool isFullScreen = aParent->GetFullScreen(); // Where should we open this? @@ -2453,7 +2509,7 @@ int32_t nsWindowWatcher::GetWindowOpenLocation(nsPIDOMWindowOuter* aParent, } if (restrictionPref == 2) { - // Only continue if there are no size/position features and no special + // Only continue if there are no width feature and no special // chrome flags - with the exception of the remoteness and private flags, // which might have been automatically flipped by Gecko. int32_t uiChromeFlags = aChromeFlags; @@ -2462,8 +2518,7 @@ int32_t nsWindowWatcher::GetWindowOpenLocation(nsPIDOMWindowOuter* aParent, nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW | nsIWebBrowserChrome::CHROME_NON_PRIVATE_WINDOW | nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME); - if (uiChromeFlags != nsIWebBrowserChrome::CHROME_ALL || - aPositionSpecified || aSizeSpecified) { + if (uiChromeFlags != nsIWebBrowserChrome::CHROME_ALL || aWidthSpecified) { return nsIBrowserDOMWindow::OPEN_NEWWINDOW; } } diff --git a/toolkit/components/windowwatcher/nsWindowWatcher.h b/toolkit/components/windowwatcher/nsWindowWatcher.h index ec7531161fe8..b6e51d95cff1 100644 --- a/toolkit/components/windowwatcher/nsWindowWatcher.h +++ b/toolkit/components/windowwatcher/nsWindowWatcher.h @@ -53,8 +53,7 @@ class nsWindowWatcher : public nsIWindowWatcher, static int32_t GetWindowOpenLocation(nsPIDOMWindowOuter* aParent, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, - bool aSizeSpecified); + bool aWidthSpecified); // Will first look for a caller on the JS stack, and then fall back on // aCurrentContext if it can't find one. @@ -86,10 +85,15 @@ class nsWindowWatcher : public nsIWindowWatcher, static nsresult URIfromURL(const char* aURL, mozIDOMWindowProxy* aParent, nsIURI** aURI); - static uint32_t CalculateChromeFlagsForChild(const nsACString& aFeaturesStr); + static bool ShouldOpenPopup(const nsACString& aFeatures, + const SizeSpec& aSizeSpec); + + static uint32_t CalculateChromeFlagsForChild(const nsACString& aFeaturesStr, + const SizeSpec& aSizeSpec); static uint32_t CalculateChromeFlagsForParent(mozIDOMWindowProxy* aParent, const nsACString& aFeaturesStr, + const SizeSpec& aSizeSpec, bool aDialog, bool aChromeURL, bool aHasChromeParent, bool aCalledFromJS); @@ -121,10 +125,9 @@ class nsWindowWatcher : public nsIWindowWatcher, static uint32_t CalculateChromeFlagsHelper(uint32_t aInitialFlags, const nsACString& aFeatures, + const SizeSpec& aSizeSpec, bool& presenceFlag, - bool aDialog = false, - bool aHasChromeParent = false, - bool aChromeURL = false); + bool aHasChromeParent = false); static uint32_t EnsureFlagsSafeForContent(uint32_t aChromeFlags, bool aChromeURL = false); diff --git a/toolkit/components/windowwatcher/test/browser.ini b/toolkit/components/windowwatcher/test/browser.ini index 925b96e341e3..a77ae1cf20fe 100644 --- a/toolkit/components/windowwatcher/test/browser.ini +++ b/toolkit/components/windowwatcher/test/browser.ini @@ -1,5 +1,7 @@ [DEFAULT] tags = openwindow +support-files = + head.js [browser_new_content_window_chromeflags.js] [browser_new_remote_window_flags.js] @@ -7,3 +9,6 @@ run-if = e10s [browser_new_content_window_from_chrome_principal.js] [browser_new_sized_window.js] skip-if = os == 'win' # Bug 1276802 - Opening windows from content on Windows might not get the size right +[browser_popup_condition.js] +skip-if = debug # Opening many windows takes long on debug build +[browser_non_popup_from_popup.js] diff --git a/toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.js b/toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.js index b73b9c1789bf..459ec51ff9cf 100644 --- a/toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.js +++ b/toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.js @@ -3,49 +3,12 @@ * being opened from content. */ -// The following features set chrome flags on new windows and are -// supported by web content. The schema for each property on this -// object is as follows: +// The following are not allowed from web content. // // : { // flag: , // defaults_to: // } -const ALLOWED = { - toolbar: { - flag: Ci.nsIWebBrowserChrome.CHROME_TOOLBAR, - defaults_to: true, - }, - personalbar: { - flag: Ci.nsIWebBrowserChrome.CHROME_PERSONAL_TOOLBAR, - defaults_to: true, - }, - menubar: { - flag: Ci.nsIWebBrowserChrome.CHROME_MENUBAR, - defaults_to: true, - }, - scrollbars: { - flag: Ci.nsIWebBrowserChrome.CHROME_SCROLLBARS, - defaults_to: false, - }, - minimizable: { - flag: Ci.nsIWebBrowserChrome.CHROME_WINDOW_MIN, - defaults_to: true, - }, -}; - -// Construct a features string that flips all ALLOWED features -// to not be their defaults. -const ALLOWED_STRING = Object.keys(ALLOWED) - .map(feature => { - let toValue = ALLOWED[feature].defaults_to ? "no" : "yes"; - return `${feature}=${toValue}`; - }) - .join(","); - -// The following are not allowed from web content, at least -// in the default case (since some are disabled by default -// via the dom.disable_window_open_feature pref branch). const DISALLOWED = { location: { flag: Ci.nsIWebBrowserChrome.CHROME_LOCATIONBAR, @@ -140,21 +103,6 @@ registerCleanupFunction(() => { Services.prefs.clearUserPref("browser.link.open_newwindow"); }); -/** - * Given some nsIDOMWindow for a window running in the parent - * process, return the nsIWebBrowserChrome chrome flags for - * the associated XUL window. - * - * @param win (nsIDOMWindow) - * Some window in the parent process. - * @returns int - */ -function getParentChromeFlags(win) { - return win.docShell.treeOwner - .QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIAppWindow).chromeFlags; -} - /** * Given some nsIDOMWindow for a window running in the parent process, * asynchronously return the nsIWebBrowserChrome chrome flags for the @@ -184,34 +132,13 @@ function getContentChromeFlags(win) { } /** - * For some chromeFlags, ensures that flags that are in the - * ALLOWED group were modified, and that flags in the DISALLOWED + * For some chromeFlags, ensures that flags in the DISALLOWED * group were not modified. * * @param chromeFlags (int) * Some chromeFlags to check. */ -function assertContentFlags(chromeFlags) { - for (let feature in ALLOWED) { - let flag = ALLOWED[feature].flag; - - if (ALLOWED[feature].defaults_to) { - // The feature is supposed to default to true, so we should - // have been able to flip it off. - Assert.ok( - !(chromeFlags & flag), - `Expected feature ${feature} to be disabled` - ); - } else { - // The feature is supposed to default to false, so we should - // have been able to flip it on. - Assert.ok( - chromeFlags & flag, - `Expected feature ${feature} to be enabled` - ); - } - } - +function assertContentFlags(chromeFlags, isPopup) { for (let feature in DISALLOWED) { let flag = DISALLOWED[feature].flag; Assert.ok(flag, "Expected flag to be a non-zeroish value"); @@ -235,11 +162,10 @@ function assertContentFlags(chromeFlags) { /** * Opens a window from content using window.open with the - * features computed from ALLOWED and DISALLOWED. The computed - * feature string attempts to flip every feature away from their - * default. + * features computed from DISALLOWED. The computed feature string attempts to + * flip every feature away from their default. */ -add_task(async function test_new_remote_window_flags() { +add_task(async function test_disallowed_flags() { // Construct a features string that flips all DISALLOWED features // to not be their defaults. const DISALLOWED_STRING = Object.keys(DISALLOWED) @@ -249,7 +175,7 @@ add_task(async function test_new_remote_window_flags() { }) .join(","); - const FEATURES = [ALLOWED_STRING, DISALLOWED_STRING].join(","); + const FEATURES = [DISALLOWED_STRING].join(","); const SCRIPT_PAGE = `data:text/html,`; const SCRIPT_PAGE_FOR_CHROME_ALL = `data:text/html,`; diff --git a/toolkit/components/windowwatcher/test/browser_non_popup_from_popup.js b/toolkit/components/windowwatcher/test/browser_non_popup_from_popup.js new file mode 100644 index 000000000000..16511d16e99b --- /dev/null +++ b/toolkit/components/windowwatcher/test/browser_non_popup_from_popup.js @@ -0,0 +1,53 @@ +"use strict"; + +// Opening non-popup from a popup should open a new tab in the most recent +// non-popup window. +add_task(async function test_non_popup_from_popup() { + const BLANK_PAGE = "data:text/html,"; + + // A page opened in a new tab. + const OPEN_PAGE = "data:text/plain,hello"; + + // A page opened in a new popup. + // This opens a new non-popup page with OPEN_PAGE, + // tha should be opened in a new tab in most recent window. + const NON_POPUP_OPENER = btoa( + `data:text/html,` + ); + + // A page opened in a new tab. + // This opens a popup with NON_POPUP_OPENER. + const POPUP_OPENER = `data:text/html,`; + + await SpecialPowers.pushPrefEnv({ + set: [["browser.link.open_newwindow", 3]], + }); + + await BrowserTestUtils.withNewTab( + { + gBrowser, + url: BLANK_PAGE, + }, + async function(browser) { + // Wait for a popup opened by POPUP_OPENER. + const newPopupPromise = BrowserTestUtils.waitForNewWindow(); + + // Wait for a new tab opened by NON_POPUP_OPENER. + const newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, OPEN_PAGE); + + // Open a new tab that opens a popup with NON_POPUP_OPENER. + await BrowserTestUtils.loadURI(gBrowser, POPUP_OPENER); + + let win = await newPopupPromise; + Assert.ok(true, "popup is opened"); + + let tab = await newTabPromise; + Assert.ok(true, "new tab is opened in the recent window"); + + BrowserTestUtils.removeTab(tab); + await BrowserTestUtils.closeWindow(win); + } + ); + + await SpecialPowers.popPrefEnv(); +}); diff --git a/toolkit/components/windowwatcher/test/browser_popup_condition.js b/toolkit/components/windowwatcher/test/browser_popup_condition.js new file mode 100644 index 000000000000..273ebdfffa5f --- /dev/null +++ b/toolkit/components/windowwatcher/test/browser_popup_condition.js @@ -0,0 +1,308 @@ +"use strict"; + +requestLongerTimeout(5); + +/** + * Opens windows from content using window.open with several features patterns. + */ +add_task(async function test_popup_conditions() { + const PATTERNS = [ + { features: "", popup: false }, + + // If features isn't empty, the following should be true to open tab/window: + // * location or toolbar (defaults to false) + // * menubar (defaults to false) + // * resizable (defaults to true) + // * scrollbars (defaults to false) + // * status (defaults to false) + // and also the following shouldn't be specified: + // * left or screenX + // * top or screenY + // * width or innerWidth + // * outerWidth + // * height or innerHeight + // * outerHeight + { features: "location,menubar,resizable,scrollbars,status", popup: false }, + { features: "toolbar,menubar,resizable,scrollbars,status", popup: false }, + { + features: "location,menubar,toolbar,resizable,scrollbars,status", + popup: false, + }, + + // resizable defaults to true. + { features: "location,menubar,scrollbars,status", popup: false }, + + // The following testcases use "location,menubar,scrollbars,status" + // as the base non-popup case, and test the boundary between popup + // vs non-popup. + + // If either location or toolbar is true, not popup. + { + features: "location=0,toolbar,menubar,resizable,scrollbars,status", + popup: false, + }, + { + features: "location,toolbar=0,menubar,resizable,scrollbars,status", + popup: false, + }, + { + features: "location,toolbar,menubar,resizable,scrollbars,status", + popup: false, + }, + + // If both location and toolbar are false, popup. + { + features: "location=0,toolbar=0,menubar,resizable,scrollbars,status", + popup: true, + }, + { features: "menubar,scrollbars,status", popup: true }, + + // If menubar is false, popup. + { features: "location,resizable,scrollbars,status", popup: true }, + { features: "location,menubar=0,resizable,scrollbars,status", popup: true }, + + // If resizable is true, not popup. + { + features: "location,menubar,resizable=yes,scrollbars,status", + popup: false, + }, + + // If resizable is false, popup. + { features: "location,menubar,resizable=0,scrollbars,status", popup: true }, + + // If scrollbars is false, popup. + { features: "location,menubar,resizable,status", popup: true }, + { features: "location,menubar,resizable,scrollbars=0,status", popup: true }, + + // If status is false, popup. + { features: "location,menubar,resizable,scrollbars", popup: true }, + { features: "location,menubar,resizable,scrollbars,status=0", popup: true }, + + // If either width or innerWidth is specified, popup. + { features: "location,menubar,scrollbars,status,width=100", popup: true }, + { + features: "location,menubar,scrollbars,status,innerWidth=100", + popup: true, + }, + + // If outerWidth is specified, popup. + { + features: "location,menubar,scrollbars,status,outerWidth=100", + popup: true, + }, + + // left or screenX alone doesn't make it a popup. + { + features: "location,menubar,scrollbars,status,left=100", + popup: false, + }, + { + features: "location,menubar,scrollbars,status,screenX=100", + popup: false, + }, + + // top or screenY alone doesn't make it a popup. + { + features: "location,menubar,scrollbars,status,top=100", + popup: false, + }, + { + features: "location,menubar,scrollbars,status,screenY=100", + popup: false, + }, + + // height or innerHeight or outerHeight alone doesn't make it a popup. + { + features: "location,menubar,scrollbars,status,height=100", + popup: false, + }, + { + features: "location,menubar,scrollbars,status,innerHeight=100", + popup: false, + }, + { + features: "location,menubar,scrollbars,status,outerHeight=100", + popup: false, + }, + + // Most feature defaults to false if the feature is not empty. + // Specifying only some of them opens a popup. + { features: "location", popup: true }, + { features: "toolbar", popup: true }, + { features: "location,toolbar", popup: true }, + { features: "menubar", popup: true }, + { features: "location,toolbar,menubar", popup: true }, + { features: "resizable", popup: true }, + { features: "scrollbars", popup: true }, + { features: "status", popup: true }, + { features: "left=500", popup: true }, + { features: "top=500", popup: true }, + { features: "left=500,top=500", popup: true }, + { features: "width=500", popup: true }, + { features: "height=500", popup: true }, + { features: "width=500, height=500", popup: true }, + + // Specifying unknown feature makes the feature not empty. + { features: "someunknownfeature", popup: true }, + + // personalbar have no effect. + { features: "personalbar=0", popup: true }, + { features: "personalbar=yes", popup: true }, + { + features: "location,menubar,resizable,scrollbars,status,personalbar=0", + popup: false, + }, + { + features: "location,menubar,resizable,scrollbars,status,personalbar=yes", + popup: false, + }, + + // noopener is removed before testing if feature is empty. + { features: "noopener", popup: false }, + + // noreferrer is removed before testing if feature is empty. + { features: "noreferrer", popup: false }, + ]; + + const WINDOW_FLAGS = { + CHROME_WINDOW_BORDERS: true, + CHROME_WINDOW_CLOSE: true, + CHROME_WINDOW_RESIZE: true, + CHROME_LOCATIONBAR: true, + CHROME_STATUSBAR: true, + CHROME_SCROLLBARS: true, + CHROME_TITLEBAR: true, + + CHROME_MENUBAR: true, + CHROME_TOOLBAR: true, + CHROME_PERSONAL_TOOLBAR: true, + }; + + const POPUP_FLAGS = { + CHROME_WINDOW_BORDERS: true, + CHROME_WINDOW_CLOSE: true, + CHROME_WINDOW_RESIZE: true, + CHROME_LOCATIONBAR: true, + CHROME_STATUSBAR: true, + CHROME_SCROLLBARS: true, + CHROME_TITLEBAR: true, + + CHROME_MENUBAR: false, + CHROME_TOOLBAR: false, + CHROME_PERSONAL_TOOLBAR: false, + }; + + async function test_patterns({ nonPopup }) { + for (const { features, popup } of PATTERNS) { + const BLANK_PAGE = "data:text/html,"; + const OPEN_PAGE = "data:text/plain,hello"; + const SCRIPT_PAGE = `data:text/html,`; + + async function testNewWindow(flags) { + await BrowserTestUtils.withNewTab( + { + gBrowser, + url: BLANK_PAGE, + }, + async function(browser) { + const newWinPromise = BrowserTestUtils.waitForNewWindow(); + await BrowserTestUtils.loadURI(gBrowser, SCRIPT_PAGE); + + const win = await newWinPromise; + const parentChromeFlags = getParentChromeFlags(win); + + for (const [name, visible] of Object.entries(flags)) { + if (visible) { + Assert.equal( + !!(parentChromeFlags & Ci.nsIWebBrowserChrome[name]), + true, + `${name} should be present for features "${features}"` + ); + } else { + Assert.equal( + !!(parentChromeFlags & Ci.nsIWebBrowserChrome[name]), + false, + `${name} should not be present for features "${features}"` + ); + } + } + + await BrowserTestUtils.closeWindow(win); + } + ); + } + + async function testNewTab() { + await BrowserTestUtils.withNewTab( + { + gBrowser, + url: BLANK_PAGE, + }, + async function(browser) { + const newTabPromise = BrowserTestUtils.waitForNewTab( + gBrowser, + OPEN_PAGE + ); + await BrowserTestUtils.loadURI(gBrowser, SCRIPT_PAGE); + + let tab = await newTabPromise; + BrowserTestUtils.removeTab(tab); + } + ); + } + + async function testCurrentTab() { + await BrowserTestUtils.withNewTab( + { + gBrowser, + url: BLANK_PAGE, + }, + async function(browser) { + const pagePromise = BrowserTestUtils.browserLoaded( + browser, + false, + OPEN_PAGE + ); + await BrowserTestUtils.loadURI(gBrowser, SCRIPT_PAGE); + + await pagePromise; + } + ); + } + + if (!popup) { + if (nonPopup == "window") { + await testNewWindow(WINDOW_FLAGS); + } else if (nonPopup == "tab") { + await testNewTab(); + } else { + // current tab + await testCurrentTab(); + } + } else { + await testNewWindow(POPUP_FLAGS); + } + } + } + + // Non-popup is opened in a new tab (default behavior). + await SpecialPowers.pushPrefEnv({ + set: [["browser.link.open_newwindow", 3]], + }); + await test_patterns({ nonPopup: "tab" }); + await SpecialPowers.popPrefEnv(); + + // Non-popup is opened in a current tab. + await SpecialPowers.pushPrefEnv({ + set: [["browser.link.open_newwindow", 1]], + }); + await test_patterns({ nonPopup: "current" }); + await SpecialPowers.popPrefEnv(); + + // Non-popup is opened in a new window. + await SpecialPowers.pushPrefEnv({ + set: [["browser.link.open_newwindow", 2]], + }); + await test_patterns({ nonPopup: "window" }); + await SpecialPowers.popPrefEnv(); +}); diff --git a/toolkit/components/windowwatcher/test/head.js b/toolkit/components/windowwatcher/test/head.js new file mode 100644 index 000000000000..911a56efac64 --- /dev/null +++ b/toolkit/components/windowwatcher/test/head.js @@ -0,0 +1,14 @@ +/** + * Given some nsIDOMWindow for a window running in the parent + * process, return the nsIWebBrowserChrome chrome flags for + * the associated XUL window. + * + * @param win (nsIDOMWindow) + * Some window in the parent process. + * @returns int + */ +function getParentChromeFlags(win) { + return win.docShell.treeOwner + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIAppWindow).chromeFlags; +} diff --git a/xpfe/appshell/nsContentTreeOwner.cpp b/xpfe/appshell/nsContentTreeOwner.cpp index 941560574930..2f4a17f5c555 100644 --- a/xpfe/appshell/nsContentTreeOwner.cpp +++ b/xpfe/appshell/nsContentTreeOwner.cpp @@ -620,9 +620,9 @@ NS_IMETHODIMP nsContentTreeOwner::SetTitle(const nsAString& aTitle) { NS_IMETHODIMP nsContentTreeOwner::ProvideWindow( mozIDOMWindowProxy* aParent, uint32_t aChromeFlags, bool aCalledFromJS, - bool aPositionSpecified, bool aSizeSpecified, nsIURI* aURI, - const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener, - bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew, + bool aWidthSpecified, nsIURI* aURI, const nsAString& aName, + const nsACString& aFeatures, bool aForceNoOpener, bool aForceNoReferrer, + nsDocShellLoadState* aLoadState, bool* aWindowIsNew, BrowsingContext** aReturn) { NS_ENSURE_ARG_POINTER(aParent); @@ -683,8 +683,7 @@ nsContentTreeOwner::ProvideWindow( } int32_t openLocation = nsWindowWatcher::GetWindowOpenLocation( - parentWin, aChromeFlags, aCalledFromJS, aPositionSpecified, - aSizeSpecified); + parentWin, aChromeFlags, aCalledFromJS, aWidthSpecified); if (openLocation != nsIBrowserDOMWindow::OPEN_NEWTAB && openLocation != nsIBrowserDOMWindow::OPEN_CURRENTWINDOW) {