Bug 1534351 - HTMLInputElement should always allow popups for system principal, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 09 Apr 2019 19:27:15 +0000
changeset 468642 13c70f92856e3e5cb8fa222e61c37c498deda5df
parent 468641 4bb96b9e1a3241c400b6fcdcd6ebf7e97fc5fcb9
child 468643 c424b65d80448819d4dd9f026560df44d93ba331
push id35845
push userncsoregi@mozilla.com
push dateWed, 10 Apr 2019 09:58:56 +0000
treeherdermozilla-central@d272180eb2fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1534351
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1534351 - HTMLInputElement should always allow popups for system principal, r=smaug Differential Revision: https://phabricator.services.mozilla.com/D26671
docshell/base/nsDocShell.cpp
dom/base/PopupBlocker.cpp
dom/base/PopupBlocker.h
dom/base/nsGlobalWindowOuter.cpp
dom/html/HTMLInputElement.cpp
dom/html/test/chrome.ini
dom/html/test/mochitest.ini
dom/html/test/test_multipleFilePicker.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9617,17 +9617,20 @@ nsresult nsDocShell::DoURILoad(nsDocShel
                           &doesNotReturnData);
       if (doesNotReturnData) {
         bool popupBlocked = true;
 
         // Let's consider external protocols as popups and let's check if the
         // page is allowed to open them without abuse regardless of allowed
         // events
         if (PopupBlocker::GetPopupControlState() <= PopupBlocker::openBlocked) {
-          popupBlocked = !PopupBlocker::TryUsePopupOpeningToken();
+          nsCOMPtr<nsINode> loadingNode =
+              mScriptGlobal->AsOuter()->GetFrameElementInternal();
+          popupBlocked = !PopupBlocker::TryUsePopupOpeningToken(
+              loadingNode ? loadingNode->NodePrincipal() : nullptr);
         } else if (mIsActive &&
                    PopupBlocker::ConsumeTimerTokenForExternalProtocolIframe()) {
           popupBlocked = false;
         } else {
           nsCOMPtr<nsINode> loadingNode =
               mScriptGlobal->AsOuter()->GetFrameElementInternal();
           if (loadingNode) {
             popupBlocked = !PopupBlocker::CanShowPopupByPermission(
--- a/dom/base/PopupBlocker.cpp
+++ b/dom/base/PopupBlocker.cpp
@@ -131,24 +131,28 @@ bool PopupBlocker::CanShowPopupByPermiss
       return false;
     }
   }
 
   return !StaticPrefs::dom_disable_open_during_load();
 }
 
 /* static */
-bool PopupBlocker::TryUsePopupOpeningToken() {
+bool PopupBlocker::TryUsePopupOpeningToken(nsIPrincipal* aPrincipal) {
   MOZ_ASSERT(sPopupStatePusherCount);
 
   if (!sUnusedPopupToken) {
     sUnusedPopupToken = true;
     return true;
   }
 
+  if (aPrincipal && nsContentUtils::IsSystemPrincipal(aPrincipal)) {
+    return true;
+  }
+
   return false;
 }
 
 /* static */
 bool PopupBlocker::IsPopupOpeningTokenUnused() { return sUnusedPopupToken; }
 
 /* static */
 void PopupBlocker::PopupStatePusherCreated() { ++sPopupStatePusherCount; }
--- a/dom/base/PopupBlocker.h
+++ b/dom/base/PopupBlocker.h
@@ -41,17 +41,19 @@ class PopupBlocker final {
 
   // This method checks if the principal is allowed by open popups by user
   // permissions. In this case, the caller should not block popups.
   static bool CanShowPopupByPermission(nsIPrincipal* aPrincipal);
 
   // This method returns true if the caller is allowed to show a popup, and it
   // consumes the popup token for the current event. There is just 1 popup
   // allowed per event.
-  static bool TryUsePopupOpeningToken();
+  // This method returns true if the token has been already consumed but
+  // aPrincipal is the system principal.
+  static bool TryUsePopupOpeningToken(nsIPrincipal* aPrincipal);
 
   static bool IsPopupOpeningTokenUnused();
 
   static PopupBlocker::PopupControlState GetEventPopupControlState(
       WidgetEvent* aEvent, Event* aDOMEvent = nullptr);
 
   // Returns if a external protocol iframe is allowed.
   static bool ConsumeTimerTokenForExternalProtocolIframe();
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -5608,17 +5608,17 @@ PopupBlocker::PopupControlState nsGlobal
       abuse = PopupBlocker::openOverridden;
   }
 
   // If this popup is allowed, let's block any other for this event, forcing
   // PopupBlocker::openBlocked state.
   if ((abuse == PopupBlocker::openAllowed ||
        abuse == PopupBlocker::openControlled) &&
       StaticPrefs::dom_block_multiple_popups() && !PopupWhitelisted() &&
-      !PopupBlocker::TryUsePopupOpeningToken()) {
+      !PopupBlocker::TryUsePopupOpeningToken(mDoc->NodePrincipal())) {
     abuse = PopupBlocker::openBlocked;
   }
 
   return abuse;
 }
 
 /* If a window open is blocked, fire the appropriate DOM events. */
 void nsGlobalWindowOuter::FireAbuseEvents(
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -649,17 +649,17 @@ bool HTMLInputElement::IsPopupBlocked() 
   nsCOMPtr<nsPIDOMWindowOuter> win = OwnerDoc()->GetWindow();
   MOZ_ASSERT(win, "window should not be null");
   if (!win) {
     return true;
   }
 
   // Check if page can open a popup without abuse regardless of allowed events
   if (PopupBlocker::GetPopupControlState() <= PopupBlocker::openBlocked) {
-    return !PopupBlocker::TryUsePopupOpeningToken();
+    return !PopupBlocker::TryUsePopupOpeningToken(OwnerDoc()->NodePrincipal());
   }
 
   return !PopupBlocker::CanShowPopupByPermission(OwnerDoc()->NodePrincipal());
 }
 
 nsresult HTMLInputElement::InitColorPicker() {
   if (mPickerRunning) {
     NS_WARNING("Just one nsIColorPicker is allowed");
--- a/dom/html/test/chrome.ini
+++ b/dom/html/test/chrome.ini
@@ -1,10 +1,9 @@
 [DEFAULT]
 support-files =
   file_anchor_ping.html
   image.png
 
 [test_anchor_ping.html]
 skip-if = os == 'android'
 [test_bug1414077.html]
-[test_multipleFilePicker.html]
 [test_external_protocol_iframe.html]
--- a/dom/html/test/mochitest.ini
+++ b/dom/html/test/mochitest.ini
@@ -607,8 +607,9 @@ skip-if = toolkit == "android"
 [test_script_module.html]
 support-files =
   file_script_module.html
   file_script_nomodule.html
 [test_getElementsByName_after_mutation.html]
 [test_bug1279218.html]
 [test_set_input_files.html]
 [test_nestediframe.html]
+[test_multipleFilePicker.html]
--- a/dom/html/test/test_multipleFilePicker.html
+++ b/dom/html/test/test_multipleFilePicker.html
@@ -1,20 +1,21 @@
 <!DOCTYPE html>
 <html>
 <head>
   <title>Test for single filepicker per event</title>
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
-  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
   <div id='foo'><a href='#'>Click here to test this issue</a></div>
   <script>
 
+SimpleTest.requestFlakyTimeout("Timeouts are needed to simulate user-interaction");
 SimpleTest.waitForExplicitFinish();
 
 let clickCount = 0;
 let foo = document.getElementById('foo');
 foo.addEventListener('click', _ => {
   if (++clickCount < 10) {
     let input = document.createElement('input');
     input.type = 'file';