Bug 1266611 - Multiple tab prompts should not overlap, r=gijs draft
authorTimothy Guan-tin Chien <timdream@gmail.com>
Sat, 21 May 2016 21:13:10 +0800
changeset 369289 9263af3a35f232ede754f512bef3d59985a9c167
parent 369022 c67dc1f9fab86d4f2cf3224307809c44fe3ce820
child 521542 d909b81ef82dc782f03532439282b2ca5125e1b3
push id18827
push userbmo:timdream@gmail.com
push dateSat, 21 May 2016 13:13:39 +0000
reviewersgijs
bugs1266611
milestone49.0a1
Bug 1266611 - Multiple tab prompts should not overlap, r=gijs MozReview-Commit-ID: 6wjdoiR38Wb
browser/base/content/browser.js
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_openPromptInBackgroundTab.js
browser/base/content/test/general/openPromptOffTimeout.html
browser/base/content/test/tabPrompts/browser.ini
browser/base/content/test/tabPrompts/browser_multiplePrompts.js
browser/base/content/test/tabPrompts/browser_openPromptInBackgroundTab.js
browser/base/content/test/tabPrompts/openPromptOffTimeout.html
browser/base/moz.build
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7829,21 +7829,27 @@ TabModalPromptBox.prototype = {
     }
     onCloseCallback.apply(this, args);
   },
 
   appendPrompt(args, onCloseCallback) {
     const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
     let newPrompt = document.createElementNS(XUL_NS, "tabmodalprompt");
     let browser = this.browser;
-    browser.parentNode.appendChild(newPrompt);
+    browser.parentNode.insertBefore(newPrompt, browser.nextSibling);
     browser.setAttribute("tabmodalPromptShowing", true);
 
     newPrompt.clientTop; // style flush to assure binding is attached
 
+    let prompts = this.listPrompts();
+    if (prompts.length > 1) {
+      // Let's hide ourself behind the current prompt.
+      newPrompt.hidden = true;
+    }
+
     let principalToAllowFocusFor = this._allowTabFocusByPromptPrincipal;
     delete this._allowTabFocusByPromptPrincipal;
 
     let allowFocusCheckbox; // Define outside the if block so we can bind it into the callback.
     let hostForAllowFocusCheckbox = "";
     try {
       hostForAllowFocusCheckbox = principalToAllowFocusFor.URI.host;
     } catch (ex) { /* Ignore exceptions for host-less URIs */ }
@@ -7868,16 +7874,17 @@ TabModalPromptBox.prototype = {
 
   removePrompt(aPrompt) {
     let browser = this.browser;
     browser.parentNode.removeChild(aPrompt);
 
     let prompts = this.listPrompts();
     if (prompts.length) {
       let prompt = prompts[prompts.length - 1];
+      prompt.hidden = false;
       prompt.Dialog.setDefaultFocus();
     } else {
       browser.removeAttribute("tabmodalPromptShowing");
       browser.focus();
     }
   },
 
   listPrompts(aPrompt) {
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -338,18 +338,16 @@ skip-if = os != "win" # The Fitts Law me
 tags = mcb
 [browser_offlineQuotaNotification.js]
 skip-if = buildapp == 'mulet'
 [browser_feed_discovery.js]
 support-files = feed_discovery.html
 [browser_gZipOfflineChild.js]
 skip-if = buildapp == 'mulet' # Bug 1066070 - I don't think either popup notifications nor addon install stuff works?
 support-files = test_offline_gzip.html gZipOfflineChild.cacheManifest gZipOfflineChild.cacheManifest^headers^ gZipOfflineChild.html gZipOfflineChild.html^headers^
-[browser_openPromptInBackgroundTab.js]
-support-files = openPromptOffTimeout.html
 [browser_overflowScroll.js]
 [browser_pageInfo.js]
 skip-if = buildapp == 'mulet'
 [browser_page_style_menu.js]
 [browser_page_style_menu_update.js]
 [browser_parsable_css.js]
 [browser_parsable_script.js]
 skip-if = asan || (os == 'linux' && !debug && (bits == 32)) # disabled on asan because of timeouts, and bug 1172468 for the linux 32-bit pgo issue.
deleted file mode 100644
--- a/browser/base/content/test/general/browser_openPromptInBackgroundTab.js
+++ /dev/null
@@ -1,63 +0,0 @@
-"use strict";
-
-const ROOT = getRootDirectory(gTestPath).replace("chrome://mochitests/content/", "http://example.com/");
-let pageWithAlert = ROOT + "openPromptOffTimeout.html";
-
-registerCleanupFunction(function() {
-  Services.perms.removeAll(makeURI(pageWithAlert));
-});
-
-/*
- * This test opens a tab that alerts when it is hidden. We then switch away
- * from the tab, and check that by default the tab is not automatically
- * re-selected. We also check that a checkbox appears in the alert that allows
- * the user to enable this automatically re-selecting. We then check that
- * checking the checkbox does actually enable that behaviour.
- */
-add_task(function*() {
-  yield pushPrefs(["browser.tabs.dontfocusfordialogs", true]);
-  let firstTab = gBrowser.selectedTab;
-  // load page that opens prompt when page is hidden
-  let openedTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, pageWithAlert, true);
-  let openedTabGotAttentionPromise = BrowserTestUtils.waitForAttribute("attention", openedTab, "true");
-  // switch away from that tab again - this triggers the alert.
-  yield BrowserTestUtils.switchTab(gBrowser, firstTab);
-  // ... but that's async on e10s...
-  yield openedTabGotAttentionPromise;
-  // check for attention attribute
-  is(openedTab.getAttribute("attention"), "true", "Tab with alert should have 'attention' attribute.");
-  ok(!openedTab.selected, "Tab with alert should not be selected");
-
-  // switch tab back, and check the checkbox is displayed:
-  yield BrowserTestUtils.switchTab(gBrowser, openedTab);
-  // check the prompt is there, and the extra row is present
-  let prompts = openedTab.linkedBrowser.parentNode.querySelectorAll("tabmodalprompt");
-  is(prompts.length, 1, "There should be 1 prompt");
-  let ourPrompt = prompts[0];
-  let row = ourPrompt.querySelector("row");
-  ok(row, "Should have found the row with our checkbox");
-  let checkbox = row.querySelector("checkbox[label*='example.com']");
-  ok(checkbox, "The checkbox should be there");
-  ok(!checkbox.checked, "Checkbox shouldn't be checked");
-  // tick box and accept dialog
-  checkbox.checked = true;
-  ourPrompt.onButtonClick(0);
-  // check permission is set
-  let ps = Services.perms;
-  is(ps.ALLOW_ACTION, ps.testPermission(makeURI(pageWithAlert), "focus-tab-by-prompt"),
-     "Tab switching should now be allowed");
-
-  let openedTabSelectedPromise = BrowserTestUtils.waitForAttribute("selected", openedTab, "true");
-  // switch to other tab again
-  yield BrowserTestUtils.switchTab(gBrowser, firstTab);
-
-  // This is sync in non-e10s, but in e10s we need to wait for this, so yield anyway.
-  // Note that the switchTab promise doesn't actually guarantee anything about *which*
-  // tab ends up as selected when its event fires, so using that here wouldn't work.
-  yield openedTabSelectedPromise;
-  // should be switched back
-  ok(openedTab.selected, "Ta-dah, the other tab should now be selected again!");
-
-  yield BrowserTestUtils.removeTab(openedTab);
-});
-
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabPrompts/browser.ini
@@ -0,0 +1,3 @@
+[browser_multiplePrompts.js]
+[browser_openPromptInBackgroundTab.js]
+support-files = openPromptOffTimeout.html
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabPrompts/browser_multiplePrompts.js
@@ -0,0 +1,66 @@
+"use strict";
+
+/*
+ * This test triggers multiple alerts on one single tab, because it"s possible
+ * for web content to do so. The behavior is described in bug 1266353.
+ *
+ * We assert the presentation of the multiple alerts, ensuring we show only
+ * the oldest one.
+ */
+add_task(function*() {
+  const PROMPTCOUNT = 5;
+
+  let contentScript = function() {
+    var i = 5; // contentScript has no access to PROMPTCOUNT.
+    window.addEventListener("message", function() {
+      i--;
+      if (i) {
+        window.postMessage("ping", "*");
+      }
+      alert("Alert countdown #" + i);
+    });
+    window.postMessage("ping", "*");
+  };
+  let url = "data:text/html,<script>(" + encodeURIComponent(contentScript.toSource()) + ")();</script>"
+
+  let promptsOpenedPromise = new Promise(function(resolve) {
+    let unopenedPromptCount = PROMPTCOUNT;
+    Services.obs.addObserver(function observer() {
+      unopenedPromptCount--;
+      if (!unopenedPromptCount) {
+        Services.obs.removeObserver(observer, "tabmodal-dialog-loaded");
+        info("Prompts opened.");
+        resolve();
+      }
+    }, "tabmodal-dialog-loaded", false);
+  });
+
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, url, true);
+  info("Tab loaded");
+
+  yield promptsOpenedPromise;
+
+  let promptsCount = PROMPTCOUNT;
+  while (promptsCount--) {
+    let prompts = tab.linkedBrowser.parentNode.querySelectorAll("tabmodalprompt");
+    is(prompts.length, promptsCount + 1, "There should be " + (promptsCount + 1) + " prompt(s).");
+    // The oldest should be the first.
+    let i = 0;
+    for (let prompt of prompts) {
+      is(prompt.Dialog.args.text, "Alert countdown #" + i, "The #" + i + " alert should be labelled as such.");
+      if (i !== promptsCount) {
+        is(prompt.hidden, true, "This prompt should be hidden.");
+        i++;
+        continue;
+      }
+
+      is(prompt.hidden, false, "The last prompt should not be hidden.");
+      prompt.onButtonClick(0);
+    }
+  }
+
+  let prompts = tab.linkedBrowser.parentNode.querySelectorAll("tabmodalprompt");
+  is(prompts.length, 0, "Prompts should all be dismissed.");
+
+  yield BrowserTestUtils.removeTab(tab);
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabPrompts/browser_openPromptInBackgroundTab.js
@@ -0,0 +1,62 @@
+"use strict";
+
+const ROOT = getRootDirectory(gTestPath).replace("chrome://mochitests/content/", "http://example.com/");
+let pageWithAlert = ROOT + "openPromptOffTimeout.html";
+
+registerCleanupFunction(function() {
+  Services.perms.removeAll(makeURI(pageWithAlert));
+});
+
+/*
+ * This test opens a tab that alerts when it is hidden. We then switch away
+ * from the tab, and check that by default the tab is not automatically
+ * re-selected. We also check that a checkbox appears in the alert that allows
+ * the user to enable this automatically re-selecting. We then check that
+ * checking the checkbox does actually enable that behaviour.
+ */
+add_task(function*() {
+  yield SpecialPowers.pushPrefEnv({"set": [["browser.tabs.dontfocusfordialogs", true]]});
+  let firstTab = gBrowser.selectedTab;
+  // load page that opens prompt when page is hidden
+  let openedTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, pageWithAlert, true);
+  let openedTabGotAttentionPromise = BrowserTestUtils.waitForAttribute("attention", openedTab, "true");
+  // switch away from that tab again - this triggers the alert.
+  yield BrowserTestUtils.switchTab(gBrowser, firstTab);
+  // ... but that's async on e10s...
+  yield openedTabGotAttentionPromise;
+  // check for attention attribute
+  is(openedTab.getAttribute("attention"), "true", "Tab with alert should have 'attention' attribute.");
+  ok(!openedTab.selected, "Tab with alert should not be selected");
+
+  // switch tab back, and check the checkbox is displayed:
+  yield BrowserTestUtils.switchTab(gBrowser, openedTab);
+  // check the prompt is there, and the extra row is present
+  let prompts = openedTab.linkedBrowser.parentNode.querySelectorAll("tabmodalprompt");
+  is(prompts.length, 1, "There should be 1 prompt");
+  let ourPrompt = prompts[0];
+  let row = ourPrompt.querySelector("row");
+  ok(row, "Should have found the row with our checkbox");
+  let checkbox = row.querySelector("checkbox[label*='example.com']");
+  ok(checkbox, "The checkbox should be there");
+  ok(!checkbox.checked, "Checkbox shouldn't be checked");
+  // tick box and accept dialog
+  checkbox.checked = true;
+  ourPrompt.onButtonClick(0);
+  // check permission is set
+  let ps = Services.perms;
+  is(ps.ALLOW_ACTION, ps.testPermission(makeURI(pageWithAlert), "focus-tab-by-prompt"),
+     "Tab switching should now be allowed");
+
+  let openedTabSelectedPromise = BrowserTestUtils.waitForAttribute("selected", openedTab, "true");
+  // switch to other tab again
+  yield BrowserTestUtils.switchTab(gBrowser, firstTab);
+
+  // This is sync in non-e10s, but in e10s we need to wait for this, so yield anyway.
+  // Note that the switchTab promise doesn't actually guarantee anything about *which*
+  // tab ends up as selected when its event fires, so using that here wouldn't work.
+  yield openedTabSelectedPromise;
+  // should be switched back
+  ok(openedTab.selected, "Ta-dah, the other tab should now be selected again!");
+
+  yield BrowserTestUtils.removeTab(openedTab);
+});
rename from browser/base/content/test/general/openPromptOffTimeout.html
rename to browser/base/content/test/tabPrompts/openPromptOffTimeout.html
--- a/browser/base/moz.build
+++ b/browser/base/moz.build
@@ -18,16 +18,17 @@ BROWSER_CHROME_MANIFESTS += [
     'content/test/alerts/browser.ini',
     'content/test/chat/browser.ini',
     'content/test/general/browser.ini',
     'content/test/newtab/browser.ini',
     'content/test/plugins/browser.ini',
     'content/test/popupNotifications/browser.ini',
     'content/test/referrer/browser.ini',
     'content/test/social/browser.ini',
+    'content/test/tabPrompts/browser.ini',
     'content/test/urlbar/browser.ini',
 ]
 
 DEFINES['MOZ_APP_VERSION'] = CONFIG['MOZ_APP_VERSION']
 DEFINES['MOZ_APP_VERSION_DISPLAY'] = CONFIG['MOZ_APP_VERSION_DISPLAY']
 
 DEFINES['APP_LICENSE_BLOCK'] = '%s/content/overrides/app-license.html' % SRCDIR