Bug 1050638 - should be able to close tab with onbeforeunload warning if clicking close a second time, r=ttaubert

--HG-- extra : rebase_source : 2290f2d27e57cf3281d2884442e54e9c29e0e4e8
2014-10-10 08:36:51 +01:00 · 2014-10-10 08:36:51 +01:00 · cefd9f3267
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@ -1162,9 +1162,15 @@
                //   3. User returns to tab, presses "Leave page"
                let promptBox = this.getTabModalPromptBox(oldBrowser);
                let prompts = promptBox.listPrompts();
-                // NB: This code assumes that the beforeunload prompt
-                //     is the top-most prompt on the tab.
-                promptBox.removePrompt(prompts[prompts.length - 1]);
+                // There might not be any prompts here if the tab was closed
+                // while in an onbeforeunload prompt, which will have
+                // destroyed aforementioned prompt already, so check there's
+                // something to remove, first:
+                if (prompts.length) {
+                  // NB: This code assumes that the beforeunload prompt
+                  //     is the top-most prompt on the tab.
+                  promptBox.removePrompt(prompts[prompts.length - 1]);
+                }
              }

              oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
@ -1938,13 +1944,11 @@
        <body>
          <![CDATA[
            if (aTab.closing ||
-                aTab._pendingPermitUnload ||
                this._windowIsClosing)
              return false;

            var browser = this.getBrowserForTab(aTab);
-
-            if (!aTabWillBeMoved) {
+            if (!aTab._pendingPermitUnload && !aTabWillBeMoved) {
              let ds = browser.docShell;
              if (ds && ds.contentViewer) {
                // We need to block while calling permitUnload() because it
@ -1953,8 +1957,12 @@
                aTab._pendingPermitUnload = true;
                let permitUnload = ds.contentViewer.permitUnload();
                delete aTab._pendingPermitUnload;
-
-                if (!permitUnload) {
+                // If we were closed during the unload, we return false now so
+                // we don't (try to) close the same tab again. Of course, we
+                // also stop if the unload was cancelled by the user:
+                if (aTab.closing || !permitUnload) {
+                  // NB: deliberately keep the _closedDuringPermitUnload set to
+                  // true so we keep exiting early in case of multiple calls.
                  return false;
                }
              }
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@ -55,6 +55,7 @@ support-files =
  file_bug970276_favicon1.ico
  file_bug970276_favicon2.ico
  file_dom_notifications.html
+  file_double_close_tab.html
  file_favicon_change.html
  file_fullscreen-window-open.html
  get_user_media.html
@ -316,6 +317,8 @@ skip-if = buildapp == 'mulet' || (os == "linux" && debug) || e10s # linux: bug 9
 skip-if = e10s # Bug 973001 - appears user media notifications only happen in the child and don't make their way to the parent?
 [browser_discovery.js]
 skip-if = e10s # Bug 918663 - DOMLinkAdded events don't make their way to chrome
+[browser_double_close_tab.js]
+skip-if = e10s
 [browser_duplicateIDs.js]
 [browser_drag.js]
 skip-if = true # browser_drag.js is disabled, as it needs to be updated for the new behavior from bug 320638.
--- a/browser/base/content/test/general/browser_double_close_tab.js
+++ b/browser/base/content/test/general/browser_double_close_tab.js
@ -0,0 +1,80 @@
+"use strict"
+const TEST_PAGE = "http://mochi.test:8888/browser/browser/base/content/test/general/file_double_close_tab.html";
+let testTab;
+
+function waitForDialog(callback) {
+  function onTabModalDialogLoaded(node) {
+    Services.obs.removeObserver(onTabModalDialogLoaded, "tabmodal-dialog-loaded");
+    callback(node);
+  }
+
+  // Listen for the dialog being created
+  Services.obs.addObserver(onTabModalDialogLoaded, "tabmodal-dialog-loaded", false);
+}
+
+function waitForDialogDestroyed(node, callback) {
+  // Now listen for the dialog going away again...
+  let observer = new MutationObserver(function(muts) {
+    if (!node.parentNode) {
+      ok(true, "Dialog is gone");
+      done();
+    }
+  });
+  observer.observe(node.parentNode, {childList: true});
+  let failureTimeout = setTimeout(function() {
+    ok(false, "Dialog should have been destroyed");
+    done();
+  }, 10000);
+
+  function done() {
+    clearTimeout(failureTimeout);
+    observer.disconnect();
+    observer = null;
+    callback();
+  }
+}
+
+add_task(function*() {
+  testTab = gBrowser.selectedTab = gBrowser.addTab();
+  yield promiseTabLoadEvent(testTab, TEST_PAGE);
+  //XXXgijs the reason this has nesting and callbacks rather than promises is
+  // that DOM promises resolve on the next tick. So they're scheduled
+  // in an event queue. So when we spin a new event queue for a modal dialog...
+  // everything gets messed up and the promise's .then callbacks never get
+  // called, despite resolve() being called just fine.
+  let dialogNode = yield new Promise(resolveOuter => {
+    waitForDialog(function(dialogNode) {
+      waitForDialogDestroyed(dialogNode, () => {
+        let doCompletion = () => setTimeout(resolveOuter, 10);
+        info("Now checking if dialog is destroyed");
+        ok(!dialogNode.parentNode, "onbeforeunload dialog should be gone.");
+        if (dialogNode.parentNode) {
+          // Failed to remove onbeforeunload dialog, so do it ourselves:
+          let leaveBtn = dialogNode.ui.button0;
+          waitForDialogDestroyed(dialogNode, doCompletion);
+          EventUtils.synthesizeMouseAtCenter(leaveBtn, {});
+          return;
+        }
+        doCompletion();
+      });
+      // Click again:
+      document.getAnonymousElementByAttribute(testTab, "anonid", "close-button").click();
+    });
+    // Click once:
+    document.getAnonymousElementByAttribute(testTab, "anonid", "close-button").click();
+  });
+  yield promiseWaitForCondition(() => !testTab.parentNode);
+  ok(!testTab.parentNode, "Tab should be closed completely");
+});
+
+registerCleanupFunction(function() {
+  if (testTab.parentNode) {
+    // Remove the handler, or closing this tab will prove tricky:
+    try {
+      testTab.linkedBrowser.contentWindow.onbeforeunload = null;
+    } catch (ex) {}
+    gBrowser.removeTab(testTab);
+  }
+});
+
+
--- a/browser/base/content/test/general/file_double_close_tab.html
+++ b/browser/base/content/test/general/file_double_close_tab.html
@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Test for bug 1050638 - clicking tab close button twice should close tab even in beforeunload case</title>
+  </head>
+  <body>
+    This page will block beforeunload. It should still be user-closable at all times.
+    <script>
+      window.onbeforeunload = function() {
+        return "stop";
+      };
+    </script>
+  </body>
+</html>