Bug 685828 - Limit displaying blocked popups in the front-end. r=Gijs
authorJohann Hofmann <jhofmann@mozilla.com>
Wed, 07 Nov 2018 21:11:41 +0000
changeset 445037 74807b4c104a2d9e5a24842bbc73746dbee508e4
parent 445036 640b4bd3443840a7be5503663048b14c0899cd93
child 445038 28f9cf6d3f5ae4dc438dd6638f37260c3a9ae472
push id35007
push useraiakab@mozilla.com
push dateThu, 08 Nov 2018 04:46:54 +0000
treeherdermozilla-central@18ea6c93e858 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs685828
milestone65.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 685828 - Limit displaying blocked popups in the front-end. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D11167
browser/base/content/browser.js
browser/base/content/test/popups/browser.ini
browser/base/content/test/popups/browser_popup_blocker.js
browser/base/content/test/popups/popup_blocker_10_popups.html
browser/locales/en-US/chrome/browser/browser.properties
modules/libpref/init/all.js
toolkit/actors/PopupBlockingChild.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -631,17 +631,23 @@ var gPopupBlockerObserver = {
 
         var stringKey = AppConstants.platform == "win"
                         ? "popupWarningButton"
                         : "popupWarningButtonUnix";
 
         var popupButtonText = gNavigatorBundle.getString(stringKey);
         var popupButtonAccesskey = gNavigatorBundle.getString(stringKey + ".accesskey");
 
-        var messageBase = gNavigatorBundle.getString("popupWarning.message");
+        let messageBase;
+        if (popupCount < this.maxReportedPopups) {
+          messageBase = gNavigatorBundle.getString("popupWarning.message");
+        } else {
+          messageBase = gNavigatorBundle.getString("popupWarning.exceeded.message");
+        }
+
         var message = PluralForm.get(popupCount, messageBase)
                                 .replace("#1", brandShortName)
                                 .replace("#2", popupCount);
 
         let notificationBox = gBrowser.getNotificationBox();
         let notification = notificationBox.getNotificationWithValue("popup-blocked");
         if (notification) {
           notification.label = message;
@@ -835,16 +841,19 @@ var gPopupBlockerObserver = {
 
   dontShowMessage() {
     var showMessage = Services.prefs.getBoolPref("privacy.popups.showBrowserMessage");
     Services.prefs.setBoolPref("privacy.popups.showBrowserMessage", !showMessage);
     gBrowser.getNotificationBox().removeCurrentNotification();
   },
 };
 
+XPCOMUtils.defineLazyPreferenceGetter(gPopupBlockerObserver, "maxReportedPopups",
+  "privacy.popups.maxReported");
+
 function gKeywordURIFixup({ target: browser, data: fixupInfo }) {
   let deserializeURI = (spec) => spec ? makeURI(spec) : null;
 
   // We get called irrespective of whether we did a keyword search, or
   // whether the original input would be vaguely interpretable as a URL,
   // so figure that out first.
   let alternativeURI = deserializeURI(fixupInfo.fixedURI);
   if (!fixupInfo.keywordProviderName || !alternativeURI || !alternativeURI.host) {
--- a/browser/base/content/test/popups/browser.ini
+++ b/browser/base/content/test/popups/browser.ini
@@ -1,14 +1,15 @@
 [browser_popupUI.js]
 [browser_popup_blocker.js]
 support-files =
   popup_blocker.html
   popup_blocker_a.html
   popup_blocker_b.html
+  popup_blocker_10_popups.html
 skip-if = (os == 'linux') || (e10s && debug) # Frequent bug 1081925 and bug 1125520 failures
 [browser_popup_frames.js]
 support-files =
   popup_blocker.html
   popup_blocker_a.html
   popup_blocker_b.html
 [browser_popup_blocker_identity_block.js]
 support-files =
--- a/browser/base/content/test/popups/browser_popup_blocker.js
+++ b/browser/base/content/test/popups/browser_popup_blocker.js
@@ -9,20 +9,43 @@ function clearAllPermissionsByPrefix(aPr
   while (perms.hasMoreElements()) {
     let perm = perms.getNext();
     if (perm.type.startsWith(aPrefix)) {
       Services.perms.removePermission(perm);
     }
   }
 }
 
-add_task(async function test_opening_blocked_popups() {
+add_task(async function setup() {
   // Enable the popup blocker.
   await SpecialPowers.pushPrefEnv({set: [["dom.disable_open_during_load", true]]});
+});
 
+// Tests that we show a special message when popup blocking exceeds
+// a certain maximum of popups per page.
+add_task(async function test_maximum_reported_blocks() {
+  Services.prefs.setIntPref("privacy.popups.maxReported", 5);
+
+  // Open the test page.
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, baseURL + "popup_blocker_10_popups.html");
+
+  // Wait for the popup-blocked notification.
+  let notification = await BrowserTestUtils.waitForCondition(() =>
+    gBrowser.getNotificationBox().getNotificationWithValue("popup-blocked"));
+
+  // Slightly hacky way to ensure we show the correct message in this case.
+  ok(notification.label.includes("more than"), "Notification label has 'more than'");
+  ok(notification.label.includes("5"), "Notification label shows the maximum number of popups");
+
+  gBrowser.removeTab(tab);
+
+  Services.prefs.clearUserPref("privacy.popups.maxReported");
+});
+
+add_task(async function test_opening_blocked_popups() {
   // Open the test page.
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, baseURL + "popup_blocker.html");
 
   // Wait for the popup-blocked notification.
   let notification;
   await BrowserTestUtils.waitForCondition(() =>
     notification = gBrowser.getNotificationBox().getNotificationWithValue("popup-blocked"));
 
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/popups/popup_blocker_10_popups.html
@@ -0,0 +1,14 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <title>Page creating ten popups</title>
+  </head>
+  <body>
+    <script type="text/javascript">
+      for (let i = 0; i < 10; i++) {
+        window.open("https://example.com");
+      }
+    </script>
+  </body>
+</html>
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -249,16 +249,21 @@ darkTheme.description=A theme with a dar
 lwthemeInstallRequest.message2=This site (%S) attempted to install a theme.
 lwthemeInstallRequest.allowButton2=Allow
 lwthemeInstallRequest.allowButton.accesskey2=a
 
 # LOCALIZATION NOTE (popupWarning.message): Semicolon-separated list of plural forms.
 # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
 # #1 is brandShortName and #2 is the number of pop-ups blocked.
 popupWarning.message=#1 prevented this site from opening a pop-up window.;#1 prevented this site from opening #2 pop-up windows.
+# LOCALIZATION NOTE (popupWarning.exceeded.message): Semicolon-separated list of plural forms.
+# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
+# The singular form is left empty for English, since the number of blocked pop-ups is always greater than 1.
+# #1 is brandShortName and #2 is the number of pop-ups blocked.
+popupWarning.exceeded.message=;#1 prevented this site from opening more than #2 pop-up windows.
 popupWarningButton=Options
 popupWarningButton.accesskey=O
 popupWarningButtonUnix=Preferences
 popupWarningButtonUnix.accesskey=P
 popupAllow=Allow pop-ups for %S
 popupBlock=Block pop-ups for %S
 popupWarningDontShowFromMessage=Don’t show this message when pop-ups are blocked
 popupShowPopupPrefix=Show ‘%S’
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1365,16 +1365,24 @@ pref("content.sink.pending_event_mode", 
 
 // Disable popups from plugins by default
 //   0 = openAllowed
 //   1 = openControlled
 //   2 = openBlocked
 //   3 = openAbused
 pref("privacy.popups.disable_from_plugins", 3);
 
+// Excessive reporting of blocked popups can be a DOS vector,
+// by overloading the main process as popups get blocked and when
+// users try to restore all popups, which is the most visible
+// option in our UI at the time of writing.
+// We will invisibly drop any popups from a page that has already
+// opened more than this number of popups.
+pref("privacy.popups.maxReported", 100);
+
 // send "do not track" HTTP header, disabled by default
 pref("privacy.donottrackheader.enabled",    false);
 // If true, close button will be shown on permission prompts
 // and for all PopupNotifications, the secondary action of
 // the popup will be called when the popup is dismissed.
 pref("privacy.permissionPrompts.showCloseButton", false);
 // Enforce tracking protection in all modes
 pref("privacy.trackingprotection.enabled",  false);
--- a/toolkit/actors/PopupBlockingChild.jsm
+++ b/toolkit/actors/PopupBlockingChild.jsm
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* eslint no-unused-vars: ["error", {args: "none"}] */
 
 var EXPORTED_SYMBOLS = ["PopupBlockingChild"];
 
 ChromeUtils.import("resource://gre/modules/ActorChild.jsm");
+ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 class PopupBlockingChild extends ActorChild {
   constructor(dispatcher) {
     super(dispatcher);
 
     this.popupData = null;
     this.popupDataInternal = null;
 
@@ -81,16 +82,21 @@ class PopupBlockingChild extends ActorCh
   }
 
   onPopupBlocked(ev) {
     if (!this.popupData) {
       this.popupData = [];
       this.popupDataInternal = [];
     }
 
+    // Avoid spamming the parent process with too many blocked popups.
+    if (this.popupData.length >= PopupBlockingChild.maxReportedPopups) {
+      return;
+    }
+
     let obj = {
       popupWindowURIspec: ev.popupWindowURI ? ev.popupWindowURI.spec : "about:blank",
       popupWindowFeatures: ev.popupWindowFeatures,
       popupWindowName: ev.popupWindowName,
     };
 
     let internals = {
       requestingWindow: ev.requestingWindow,
@@ -130,8 +136,11 @@ class PopupBlockingChild extends ActorCh
   updateBlockedPopups(freshPopup) {
     this.mm.sendAsyncMessage("PopupBlocking:UpdateBlockedPopups",
       {
         count: this.popupData ? this.popupData.length : 0,
         freshPopup,
       });
   }
 }
+
+XPCOMUtils.defineLazyPreferenceGetter(PopupBlockingChild, "maxReportedPopups",
+  "privacy.popups.maxReported");