Bug 1701001 - Part 1: Standardize the condition for opening popup from window.open. r=smaug

Removed "width" feature from the popup condition, and removed related parameters
(aWidthSpecified, and aSizeSpec) from functions.

Also added "popup" feature that explicitly specify whether to request popup or
not.
This is only for content context, and it behaves differently than existing
"popup" feature for chrome context that makes the window no-style.

Differential Revision: https://phabricator.services.mozilla.com/D129410
This commit is contained in:
Tooru Fujisawa 2021-11-06 01:19:14 +00:00
parent 1569812676
commit f11f62dcaf
11 changed files with 72 additions and 94 deletions

View file

@ -816,9 +816,9 @@ BrowserChild::GetInterface(const nsIID& aIID, void** aSink) {
NS_IMETHODIMP
BrowserChild::ProvideWindow(nsIOpenWindowInfo* aOpenWindowInfo,
uint32_t aChromeFlags, bool aCalledFromJS,
bool aWidthSpecified, nsIURI* aURI,
const nsAString& aName, const nsACString& aFeatures,
bool aForceNoOpener, bool aForceNoReferrer,
nsIURI* aURI, const nsAString& aName,
const nsACString& aFeatures, bool aForceNoOpener,
bool aForceNoReferrer,
nsDocShellLoadState* aLoadState, bool* aWindowIsNew,
BrowsingContext** aReturn) {
*aReturn = nullptr;
@ -826,7 +826,7 @@ BrowserChild::ProvideWindow(nsIOpenWindowInfo* aOpenWindowInfo,
RefPtr<BrowsingContext> parent = aOpenWindowInfo->GetParent();
int32_t openLocation = nsWindowWatcher::GetWindowOpenLocation(
parent->GetDOMWindow(), aChromeFlags, aCalledFromJS, aWidthSpecified,
parent->GetDOMWindow(), aChromeFlags, aCalledFromJS,
aOpenWindowInfo->GetIsForPrinting());
// If it turns out we're opening in the current browser, just hand over the
@ -849,9 +849,9 @@ BrowserChild::ProvideWindow(nsIOpenWindowInfo* aOpenWindowInfo,
// code back to our caller.
ContentChild* cc = ContentChild::GetSingleton();
return cc->ProvideWindowCommon(this, aOpenWindowInfo, aChromeFlags,
aCalledFromJS, aWidthSpecified, aURI, aName,
aFeatures, aForceNoOpener, aForceNoReferrer,
aLoadState, aWindowIsNew, aReturn);
aCalledFromJS, aURI, aName, aFeatures,
aForceNoOpener, aForceNoReferrer, aLoadState,
aWindowIsNew, aReturn);
}
void BrowserChild::DestroyWindow() {

View file

@ -966,10 +966,10 @@ static nsresult GetCreateWindowParams(nsIOpenWindowInfo* aOpenWindowInfo,
nsresult ContentChild::ProvideWindowCommon(
BrowserChild* aTabOpener, nsIOpenWindowInfo* aOpenWindowInfo,
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) {
uint32_t aChromeFlags, bool aCalledFromJS, nsIURI* aURI,
const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener,
bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew,
BrowsingContext** aReturn) {
MOZ_DIAGNOSTIC_ASSERT(aTabOpener, "We must have a tab opener");
*aReturn = nullptr;
@ -1037,9 +1037,9 @@ nsresult ContentChild::ProvideWindowCommon(
MOZ_DIAGNOSTIC_ASSERT(!nsContentUtils::IsSpecialName(name));
Unused << SendCreateWindowInDifferentProcess(
aTabOpener, parent, aChromeFlags, aCalledFromJS, aWidthSpecified,
aURI, features, fullZoom, name, triggeringPrincipal, csp,
referrerInfo, aOpenWindowInfo->GetOriginAttributes());
aTabOpener, parent, aChromeFlags, aCalledFromJS, aURI, features,
fullZoom, name, triggeringPrincipal, csp, referrerInfo,
aOpenWindowInfo->GetOriginAttributes());
// We return NS_ERROR_ABORT, so that the caller knows that we've abandoned
// the window open as far as it is concerned.
@ -1239,7 +1239,7 @@ nsresult ContentChild::ProvideWindowCommon(
}
SendCreateWindow(aTabOpener, parent, newChild, aChromeFlags, aCalledFromJS,
aWidthSpecified, aOpenWindowInfo->GetIsForPrinting(),
aOpenWindowInfo->GetIsForPrinting(),
aOpenWindowInfo->GetIsForWindowDotPrint(), aURI, features,
fullZoom, Principal(triggeringPrincipal), csp, referrerInfo,
aOpenWindowInfo->GetOriginAttributes(), std::move(resolve),

View file

@ -113,11 +113,10 @@ class ContentChild final : public PContentChild,
MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult ProvideWindowCommon(
BrowserChild* aTabOpener, nsIOpenWindowInfo* aOpenWindowInfo,
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);
uint32_t aChromeFlags, bool aCalledFromJS, nsIURI* aURI,
const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener,
bool aForceNoReferrer, nsDocShellLoadState* aLoadState,
bool* aWindowIsNew, BrowsingContext** aReturn);
void Init(base::ProcessId aParentPid, const char* aParentBuildID,
mozilla::ipc::ScopedPort aPort, uint64_t aChildID,

View file

@ -5129,9 +5129,8 @@ bool ContentParent::DeallocPWebBrowserPersistDocumentParent(
mozilla::ipc::IPCResult ContentParent::CommonCreateWindow(
PBrowserParent* aThisTab, BrowsingContext* aParent, bool aSetOpener,
const uint32_t& aChromeFlags, const bool& aCalledFromJS,
const bool& aWidthSpecified, const bool& aForPrinting,
const bool& aForWindowDotPrint, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom,
const bool& aForPrinting, const bool& aForWindowDotPrint,
nsIURI* aURIToLoad, const nsCString& aFeatures, const float& aFullZoom,
BrowserParent* aNextRemoteBrowser, const nsString& aName, nsresult& aResult,
nsCOMPtr<nsIRemoteTab>& aNewRemoteTab, bool* aWindowIsNew,
int32_t& aOpenLocation, nsIPrincipal* aTriggeringPrincipal,
@ -5219,7 +5218,7 @@ mozilla::ipc::IPCResult ContentParent::CommonCreateWindow(
}
aOpenLocation = nsWindowWatcher::GetWindowOpenLocation(
outerWin, aChromeFlags, aCalledFromJS, aWidthSpecified, aForPrinting);
outerWin, aChromeFlags, aCalledFromJS, aForPrinting);
MOZ_ASSERT(aOpenLocation == nsIBrowserDOMWindow::OPEN_NEWTAB ||
aOpenLocation == nsIBrowserDOMWindow::OPEN_NEWWINDOW ||
@ -5347,8 +5346,8 @@ mozilla::ipc::IPCResult ContentParent::CommonCreateWindow(
mozilla::ipc::IPCResult ContentParent::RecvCreateWindow(
PBrowserParent* aThisTab, const MaybeDiscarded<BrowsingContext>& aParent,
PBrowserParent* aNewTab, const uint32_t& aChromeFlags,
const bool& aCalledFromJS, const bool& aWidthSpecified,
const bool& aForPrinting, const bool& aForPrintPreview, nsIURI* aURIToLoad,
const bool& aCalledFromJS, const bool& aForPrinting,
const bool& aForPrintPreview, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom,
const IPC::Principal& aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp,
nsIReferrerInfo* aReferrerInfo, const OriginAttributes& aOriginAttributes,
@ -5432,10 +5431,10 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindow(
int32_t openLocation = nsIBrowserDOMWindow::OPEN_NEWWINDOW;
mozilla::ipc::IPCResult ipcResult = CommonCreateWindow(
aThisTab, parent, newBCOpenerId != 0, aChromeFlags, aCalledFromJS,
aWidthSpecified, aForPrinting, aForPrintPreview, aURIToLoad, aFeatures,
aFullZoom, newTab, VoidString(), rv, newRemoteTab, &cwi.windowOpened(),
openLocation, aTriggeringPrincipal, aReferrerInfo, /* aLoadUri = */ false,
aCsp, aOriginAttributes);
aForPrinting, aForPrintPreview, aURIToLoad, aFeatures, aFullZoom, newTab,
VoidString(), rv, newRemoteTab, &cwi.windowOpened(), openLocation,
aTriggeringPrincipal, aReferrerInfo, /* aLoadUri = */ false, aCsp,
aOriginAttributes);
if (!ipcResult) {
return ipcResult;
}
@ -5463,9 +5462,8 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindow(
mozilla::ipc::IPCResult ContentParent::RecvCreateWindowInDifferentProcess(
PBrowserParent* aThisTab, const MaybeDiscarded<BrowsingContext>& aParent,
const uint32_t& aChromeFlags, const bool& aCalledFromJS,
const bool& aWidthSpecified, nsIURI* aURIToLoad, const nsCString& aFeatures,
const float& aFullZoom, const nsString& aName,
const uint32_t& aChromeFlags, const bool& aCalledFromJS, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom, const nsString& aName,
nsIPrincipal* aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp,
nsIReferrerInfo* aReferrerInfo, const OriginAttributes& aOriginAttributes) {
MOZ_DIAGNOSTIC_ASSERT(!nsContentUtils::IsSpecialName(aName));
@ -5508,7 +5506,7 @@ mozilla::ipc::IPCResult ContentParent::RecvCreateWindowInDifferentProcess(
nsresult rv;
mozilla::ipc::IPCResult ipcResult = CommonCreateWindow(
aThisTab, parent, /* aSetOpener = */ false, aChromeFlags, aCalledFromJS,
aWidthSpecified, /* aForPrinting = */ false,
/* aForPrinting = */ false,
/* aForPrintPreview = */ false, aURIToLoad, aFeatures, aFullZoom,
/* aNextRemoteBrowser = */ nullptr, aName, rv, newRemoteTab, &windowIsNew,
openLocation, aTriggeringPrincipal, aReferrerInfo,

View file

@ -518,9 +518,8 @@ class ContentParent final
PBrowserParent* aThisBrowserParent,
const MaybeDiscarded<BrowsingContext>& aParent, PBrowserParent* aNewTab,
const uint32_t& aChromeFlags, const bool& aCalledFromJS,
const bool& aWidthSpecified, const bool& aForPrinting,
const bool& aForWindowDotPrint, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom,
const bool& aForPrinting, const bool& aForWindowDotPrint,
nsIURI* aURIToLoad, const nsCString& aFeatures, const float& aFullZoom,
const IPC::Principal& aTriggeringPrincipal,
nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo,
const OriginAttributes& aOriginAttributes,
@ -529,10 +528,9 @@ class ContentParent final
mozilla::ipc::IPCResult RecvCreateWindowInDifferentProcess(
PBrowserParent* aThisTab, const MaybeDiscarded<BrowsingContext>& aParent,
const uint32_t& aChromeFlags, const bool& aCalledFromJS,
const bool& aWidthSpecified, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom, const nsString& aName,
nsIPrincipal* aTriggeringPrincipal, nsIContentSecurityPolicy* aCsp,
nsIReferrerInfo* aReferrerInfo,
nsIURI* aURIToLoad, const nsCString& aFeatures, const float& aFullZoom,
const nsString& aName, nsIPrincipal* aTriggeringPrincipal,
nsIContentSecurityPolicy* aCsp, nsIReferrerInfo* aReferrerInfo,
const OriginAttributes& aOriginAttributes);
static void BroadcastBlobURLRegistration(
@ -764,9 +762,8 @@ class ContentParent final
mozilla::ipc::IPCResult CommonCreateWindow(
PBrowserParent* aThisTab, BrowsingContext* aParent, bool aSetOpener,
const uint32_t& aChromeFlags, const bool& aCalledFromJS,
const bool& aWidthSpecified, const bool& aForPrinting,
const bool& aForWindowDotPrint, nsIURI* aURIToLoad,
const nsCString& aFeatures, const float& aFullZoom,
const bool& aForPrinting, const bool& aForWindowDotPrint,
nsIURI* aURIToLoad, const nsCString& aFeatures, const float& aFullZoom,
BrowserParent* aNextRemoteBrowser, const nsString& aName,
nsresult& aResult, nsCOMPtr<nsIRemoteTab>& aNewRemoteTab,
bool* aWindowIsNew, int32_t& aOpenLocation,

View file

@ -1476,7 +1476,6 @@ parent:
PBrowser aNewTab,
uint32_t aChromeFlags,
bool aCalledFromJS,
bool aWidthSpecified,
bool aForPrinting,
bool aForWindowDotPrint,
nsIURI aURIToLoad,
@ -1493,7 +1492,6 @@ parent:
MaybeDiscardedBrowsingContext aParent,
uint32_t aChromeFlags,
bool aCalledFromJS,
bool aWidthSpecified,
nsIURI aURIToLoad,
nsCString aFeatures,
float aFullZoom,

View file

@ -47,9 +47,6 @@ interface nsIWindowProvider : nsISupports
* window if this provider returns null. See nsIWebBrowserChrome for
* the possible values of this field.
*
* @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
* window it returns. This URI is provided solely to help the
@ -97,7 +94,6 @@ interface nsIWindowProvider : nsISupports
BrowsingContext provideWindow(in nsIOpenWindowInfo aOpenWindowInfo,
in unsigned long aChromeFlags,
in boolean aCalledFromJS,
in boolean aWidthSpecified,
in nsIURI aURI,
in AString aName,
in AUTF8String aFeatures,

View file

@ -520,7 +520,7 @@ nsWindowWatcher::OpenWindowWithRemoteTab(nsIRemoteTab* aRemoteTab,
SizeSpec sizeSpec;
CalcSizeSpec(features, false, sizeSpec);
uint32_t chromeFlags = CalculateChromeFlagsForContent(features, sizeSpec);
uint32_t chromeFlags = CalculateChromeFlagsForContent(features);
if (isPrivateBrowsingWindow) {
chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW;
@ -707,7 +707,7 @@ nsresult nsWindowWatcher::OpenWindowInternal(
} else {
MOZ_DIAGNOSTIC_ASSERT(parentBC && parentBC->IsContent(),
"content caller must provide content parent");
chromeFlags = CalculateChromeFlagsForContent(features, sizeSpec);
chromeFlags = CalculateChromeFlagsForContent(features);
if (aDialog) {
MOZ_ASSERT(XRE_IsParentProcess());
@ -839,11 +839,10 @@ nsresult nsWindowWatcher::OpenWindowInternal(
nsCOMPtr<nsIWindowProvider> provider = do_GetInterface(parentTreeOwner);
if (provider) {
rv = provider->ProvideWindow(openWindowInfo, chromeFlags, aCalledFromJS,
sizeSpec.WidthSpecified(), uriToLoad, name,
featuresStr, aForceNoOpener,
aForceNoReferrer, aLoadState, &windowIsNew,
getter_AddRefs(newBC));
rv = provider->ProvideWindow(
openWindowInfo, chromeFlags, aCalledFromJS, uriToLoad, name,
featuresStr, aForceNoOpener, aForceNoReferrer, aLoadState,
&windowIsNew, getter_AddRefs(newBC));
if (NS_SUCCEEDED(rv) && newBC) {
nsCOMPtr<nsIDocShell> newDocShell = newBC->GetDocShell();
@ -1723,14 +1722,17 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsHelper(
}
// static
bool nsWindowWatcher::ShouldOpenPopup(const WindowFeatures& aFeatures,
const SizeSpec& aSizeSpec) {
bool nsWindowWatcher::ShouldOpenPopup(const WindowFeatures& aFeatures) {
if (aFeatures.IsEmpty()) {
return false;
}
// Follow Google Chrome's behavior that opens a popup depending on
// the following features.
// NOTE: This is different than chrome-only "popup" feature that is handled
// in nsWindowWatcher::CalculateChromeFlagsForSystem.
if (aFeatures.Exists("popup")) {
return aFeatures.GetBool("popup");
}
if (!aFeatures.GetBoolWithDefault("location", false) &&
!aFeatures.GetBoolWithDefault("toolbar", false)) {
return true;
@ -1752,11 +1754,6 @@ bool nsWindowWatcher::ShouldOpenPopup(const WindowFeatures& aFeatures,
return true;
}
// Follow Safari's behavior that opens a popup when width is specified.
if (aSizeSpec.WidthSpecified()) {
return true;
}
return false;
}
@ -1765,13 +1762,12 @@ bool nsWindowWatcher::ShouldOpenPopup(const WindowFeatures& aFeatures,
* 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::CalculateChromeFlagsForContent(
const WindowFeatures& aFeatures, const SizeSpec& aSizeSpec) {
if (aFeatures.IsEmpty() || !ShouldOpenPopup(aFeatures, aSizeSpec)) {
const WindowFeatures& aFeatures) {
if (aFeatures.IsEmpty() || !ShouldOpenPopup(aFeatures)) {
// Open the current/new tab in the current/new window
// (depends on browser.link.open_newwindow).
return nsIWebBrowserChrome::CHROME_ALL;
@ -2404,7 +2400,6 @@ void nsWindowWatcher::SizeOpenedWindow(nsIDocShellTreeOwner* aTreeOwner,
int32_t nsWindowWatcher::GetWindowOpenLocation(nsPIDOMWindowOuter* aParent,
uint32_t aChromeFlags,
bool aCalledFromJS,
bool aWidthSpecified,
bool aIsForPrinting) {
// These windows are not actually visible to the user, so we return the thing
// that we can always handle.
@ -2460,16 +2455,16 @@ int32_t nsWindowWatcher::GetWindowOpenLocation(nsPIDOMWindowOuter* aParent,
}
if (restrictionPref == 2) {
// 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.
// Only continue if there is 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;
uiChromeFlags &= ~(nsIWebBrowserChrome::CHROME_REMOTE_WINDOW |
nsIWebBrowserChrome::CHROME_FISSION_WINDOW |
nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW |
nsIWebBrowserChrome::CHROME_NON_PRIVATE_WINDOW |
nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME);
if (uiChromeFlags != nsIWebBrowserChrome::CHROME_ALL || aWidthSpecified) {
if (uiChromeFlags != nsIWebBrowserChrome::CHROME_ALL) {
return nsIBrowserDOMWindow::OPEN_NEWWINDOW;
}
}

View file

@ -54,8 +54,7 @@ class nsWindowWatcher : public nsIWindowWatcher,
static int32_t GetWindowOpenLocation(nsPIDOMWindowOuter* aParent,
uint32_t aChromeFlags,
bool aCalledFromJS, bool aWidthSpecified,
bool aIsForPrinting);
bool aCalledFromJS, bool aIsForPrinting);
// Will first look for a caller on the JS stack, and then fall back on
// aCurrentContext if it can't find one.
@ -88,11 +87,10 @@ class nsWindowWatcher : public nsIWindowWatcher,
static nsresult URIfromURL(const nsACString& aURL,
mozIDOMWindowProxy* aParent, nsIURI** aURI);
static bool ShouldOpenPopup(const mozilla::dom::WindowFeatures& aFeatures,
const SizeSpec& aSizeSpec);
static bool ShouldOpenPopup(const mozilla::dom::WindowFeatures& aFeatures);
static uint32_t CalculateChromeFlagsForContent(
const mozilla::dom::WindowFeatures& aFeatures, const SizeSpec& aSizeSpec);
const mozilla::dom::WindowFeatures& aFeatures);
static uint32_t CalculateChromeFlagsForSystem(
const mozilla::dom::WindowFeatures& aFeatures, bool aDialog,

View file

@ -15,11 +15,6 @@ add_task(async function test_popup_conditions() {
// * 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
// * height or innerHeight
{ features: "location,menubar,resizable,scrollbars,status", popup: false },
{ features: "toolbar,menubar,resizable,scrollbars,status", popup: false },
{
@ -76,11 +71,11 @@ add_task(async function test_popup_conditions() {
{ 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 },
// width and innerWidth have no effect.
{ features: "location,menubar,scrollbars,status,width=100", popup: false },
{
features: "location,menubar,scrollbars,status,innerWidth=100",
popup: true,
popup: false,
},
// outerWidth has no effect.

View file

@ -519,12 +519,14 @@ NS_IMETHODIMP nsContentTreeOwner::SetTitle(const nsAString& aTitle) {
// nsContentTreeOwner: nsIWindowProvider
//*****************************************************************************
NS_IMETHODIMP
nsContentTreeOwner::ProvideWindow(
nsIOpenWindowInfo* aOpenWindowInfo, uint32_t aChromeFlags,
bool aCalledFromJS, bool aWidthSpecified, nsIURI* aURI,
const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener,
bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew,
dom::BrowsingContext** aReturn) {
nsContentTreeOwner::ProvideWindow(nsIOpenWindowInfo* aOpenWindowInfo,
uint32_t aChromeFlags, bool aCalledFromJS,
nsIURI* aURI, const nsAString& aName,
const nsACString& aFeatures,
bool aForceNoOpener, bool aForceNoReferrer,
nsDocShellLoadState* aLoadState,
bool* aWindowIsNew,
dom::BrowsingContext** aReturn) {
NS_ENSURE_ARG_POINTER(aOpenWindowInfo);
RefPtr<dom::BrowsingContext> parent = aOpenWindowInfo->GetParent();
@ -545,7 +547,7 @@ nsContentTreeOwner::ProvideWindow(
#endif
int32_t openLocation = nsWindowWatcher::GetWindowOpenLocation(
parent->GetDOMWindow(), aChromeFlags, aCalledFromJS, aWidthSpecified,
parent->GetDOMWindow(), aChromeFlags, aCalledFromJS,
aOpenWindowInfo->GetIsForPrinting());
if (openLocation != nsIBrowserDOMWindow::OPEN_NEWTAB &&