From 15698126762fa58734d3ff6207ec761241bad0b5 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Sat, 6 Nov 2021 01:19:13 +0000 Subject: [PATCH] Bug 1737832 - Simplify the chromeFlag calculation in window.open for nsGlobalWindowOuter::Print and nsDocShell::PerformRetargeting. r=smaug This simplifies the chromeFlag calculation for: * chrome-priv case, by removing not-fully-chrome-priv case * content case, by removing presenceFlag calculation both by removing the special case for nsGlobalWindowOuter::Print and nsDocShell::PerformRetargeting, that has chrome caller but no chrome parent. Code path for nsGlobalWindowOuter::Print and nsDocShell::PerformRetargeting now uses the content case instead of the chrome-priv case, but the resulting flag doesn't change (CHROME_ALL) for those 2 consumers. Differential Revision: https://phabricator.services.mozilla.com/D129530 --- .../browser/nsIWebBrowserChrome.idl | 5 ++ .../windowwatcher/nsWindowWatcher.cpp | 83 +++---------------- .../windowwatcher/nsWindowWatcher.h | 9 +- 3 files changed, 21 insertions(+), 76 deletions(-) diff --git a/toolkit/components/browser/nsIWebBrowserChrome.idl b/toolkit/components/browser/nsIWebBrowserChrome.idl index 1e9bea1655af..4f7337926efb 100644 --- a/toolkit/components/browser/nsIWebBrowserChrome.idl +++ b/toolkit/components/browser/nsIWebBrowserChrome.idl @@ -91,6 +91,11 @@ interface nsIWebBrowserChrome : nsISupports const unsigned long CHROME_ALL = 0x00000ffe; + const unsigned long CHROME_MINIMAL_POPUP = + CHROME_WINDOW_BORDERS | CHROME_WINDOW_CLOSE | CHROME_WINDOW_RESIZE | + CHROME_LOCATIONBAR | CHROME_STATUSBAR | CHROME_SCROLLBARS | + CHROME_TITLEBAR | CHROME_WINDOW_MIN; + /** * The chrome flags for this browser chrome. The implementation should * reflect the value of this attribute by hiding or showing its chrome diff --git a/toolkit/components/windowwatcher/nsWindowWatcher.cpp b/toolkit/components/windowwatcher/nsWindowWatcher.cpp index f79bf206cfc3..b703399db5e2 100644 --- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp +++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp @@ -701,9 +701,9 @@ nsresult nsWindowWatcher::OpenWindowInternal( // callee context onto the context stack so that // the calculation sees the actual caller when doing its // security checks. - if (isCallerChrome && XRE_IsParentProcess()) { - chromeFlags = CalculateChromeFlagsForSystem( - features, sizeSpec, aDialog, uriToLoadIsChrome, hasChromeParent); + if (hasChromeParent && isCallerChrome && XRE_IsParentProcess()) { + chromeFlags = + CalculateChromeFlagsForSystem(features, aDialog, uriToLoadIsChrome); } else { MOZ_DIAGNOSTIC_ASSERT(parentBC && parentBC->IsContent(), "content caller must provide content parent"); @@ -1684,7 +1684,7 @@ nsresult nsWindowWatcher::URIfromURL(const nsACString& aURL, // static uint32_t nsWindowWatcher::CalculateChromeFlagsHelper( uint32_t aInitialFlags, const WindowFeatures& aFeatures, - const SizeSpec& aSizeSpec, bool* presenceFlag, bool aHasChromeParent) { + bool* presenceFlag) { uint32_t chromeFlags = aInitialFlags; if (aFeatures.GetBoolWithDefault("titlebar", false, presenceFlag)) { @@ -1719,57 +1719,7 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsHelper( chromeFlags |= nsIWebBrowserChrome::CHROME_SCROLLBARS; } - 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 -uint32_t nsWindowWatcher::EnsureFlagsSafeForContent(uint32_t aChromeFlags, - bool aChromeURL) { - aChromeFlags |= nsIWebBrowserChrome::CHROME_TITLEBAR; - aChromeFlags |= nsIWebBrowserChrome::CHROME_WINDOW_CLOSE; - aChromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_LOWERED; - aChromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_RAISED; - aChromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_POPUP; - /* Untrusted script is allowed to pose modal windows with a chrome - scheme. This check could stand to be better. But it effectively - prevents untrusted script from opening modal windows in general - while still allowing alerts and the like. */ - if (!aChromeURL) { - aChromeFlags &= ~(nsIWebBrowserChrome::CHROME_MODAL | - nsIWebBrowserChrome::CHROME_OPENAS_CHROME); - } - - if (!(aChromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME)) { - aChromeFlags &= ~nsIWebBrowserChrome::CHROME_DEPENDENT; - } - - return aChromeFlags; + return chromeFlags; } // static @@ -1821,14 +1771,14 @@ bool nsWindowWatcher::ShouldOpenPopup(const WindowFeatures& aFeatures, // static uint32_t nsWindowWatcher::CalculateChromeFlagsForContent( const WindowFeatures& aFeatures, const SizeSpec& aSizeSpec) { - if (aFeatures.IsEmpty()) { + if (aFeatures.IsEmpty() || !ShouldOpenPopup(aFeatures, aSizeSpec)) { + // Open the current/new tab in the current/new window + // (depends on browser.link.open_newwindow). return nsIWebBrowserChrome::CHROME_ALL; } - uint32_t chromeFlags = CalculateChromeFlagsHelper( - nsIWebBrowserChrome::CHROME_WINDOW_BORDERS, aFeatures, aSizeSpec); - - return EnsureFlagsSafeForContent(chromeFlags); + // Open a minimal popup. + return nsIWebBrowserChrome::CHROME_MINIMAL_POPUP; } /** @@ -1837,13 +1787,11 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsForContent( * @param aFeatures a string containing a list of named chrome features * @param aDialog affects the assumptions made about unnamed features * @param aChromeURL true if the window is being sent to a chrome:// URL - * @param aHasChromeParent true if the parent window is privileged * @return the chrome bitmask */ // static uint32_t nsWindowWatcher::CalculateChromeFlagsForSystem( - const WindowFeatures& aFeatures, const SizeSpec& aSizeSpec, bool aDialog, - bool aChromeURL, bool aHasChromeParent) { + const WindowFeatures& aFeatures, bool aDialog, bool aChromeURL) { MOZ_ASSERT(XRE_IsParentProcess()); MOZ_ASSERT(nsContentUtils::LegacyIsCallerChromeOrNativeCode()); @@ -1875,8 +1823,8 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsForSystem( } /* Next, allow explicitly named options to override the initial settings */ - chromeFlags = CalculateChromeFlagsHelper(chromeFlags, aFeatures, aSizeSpec, - &presenceFlag, aHasChromeParent); + chromeFlags = + CalculateChromeFlagsHelper(chromeFlags, aFeatures, &presenceFlag); // Determine whether the window is a private browsing window if (aFeatures.GetBoolWithDefault("private", false, &presenceFlag)) { @@ -2002,11 +1950,6 @@ uint32_t nsWindowWatcher::CalculateChromeFlagsForSystem( chromeFlags->copy_history */ - // Check security state for use in determing window dimensions - if (!aHasChromeParent) { - chromeFlags = EnsureFlagsSafeForContent(chromeFlags, aChromeURL); - } - return chromeFlags; } diff --git a/toolkit/components/windowwatcher/nsWindowWatcher.h b/toolkit/components/windowwatcher/nsWindowWatcher.h index 2d647fb16005..b51154aeb11b 100644 --- a/toolkit/components/windowwatcher/nsWindowWatcher.h +++ b/toolkit/components/windowwatcher/nsWindowWatcher.h @@ -95,8 +95,8 @@ class nsWindowWatcher : public nsIWindowWatcher, const mozilla::dom::WindowFeatures& aFeatures, const SizeSpec& aSizeSpec); static uint32_t CalculateChromeFlagsForSystem( - const mozilla::dom::WindowFeatures& aFeatures, const SizeSpec& aSizeSpec, - bool aDialog, bool aChromeURL, bool aHasChromeParent); + const mozilla::dom::WindowFeatures& aFeatures, bool aDialog, + bool aChromeURL); /* Compute the right SizeSpec based on aFeatures */ static void CalcSizeSpec(const mozilla::dom::WindowFeatures& aFeatures, @@ -117,10 +117,7 @@ class nsWindowWatcher : public nsIWindowWatcher, static uint32_t CalculateChromeFlagsHelper( uint32_t aInitialFlags, const mozilla::dom::WindowFeatures& aFeatures, - const SizeSpec& aSizeSpec, bool* presenceFlag = nullptr, - bool aHasChromeParent = false); - static uint32_t EnsureFlagsSafeForContent(uint32_t aChromeFlags, - bool aChromeURL = false); + bool* presenceFlag = nullptr); protected: nsTArray mEnumeratorList;