Bug 1044600 - Close subdialogs when the back button is used in the in-content preferences. r=Gijs

browser/components/preferences/in-content/subdialogs.js

@@ -6,6 +6,8 @@
 
 let gSubDialog = {
   _closingCallback: null,
+  _closingEvent: null,
+  _isClosing: false,
   _frame: null,
   _overlay: null,
   _box: null,
@@ -22,21 +24,23 @@ let gSubDialog = {
 
     // Make the close button work.
     let dialogClose = document.getElementById("dialogClose");
-    dialogClose.addEventListener("command", this.close.bind(this));
+    dialogClose.addEventListener("command", this);
 
     // DOMTitleChanged isn't fired on the frame, only on the chromeEventHandler
     let chromeBrowser = window.QueryInterface(Ci.nsIInterfaceRequestor)
                               .getInterface(Ci.nsIWebNavigation)
                               .QueryInterface(Ci.nsIDocShell)
                               .chromeEventHandler;
-    chromeBrowser.addEventListener("DOMTitleChanged", this.updateTitle, true);
+    chromeBrowser.addEventListener("DOMTitleChanged", this, true);
 
     // Similarly DOMFrameContentLoaded only fires on the top window
-    window.addEventListener("DOMFrameContentLoaded", this._onContentLoaded.bind(this), true);
+    window.addEventListener("DOMFrameContentLoaded", this, true);
 
     // Wait for the stylesheets injected during DOMContentLoaded to load before showing the dialog
     // otherwise there is a flicker of the stylesheet applying.
-    this._frame.addEventListener("load", this._onLoad.bind(this));
+    this._frame.addEventListener("load", this);
+
+    chromeBrowser.addEventListener("unload", this, true);
   },
 
   uninit: function() {
@@ -44,7 +48,7 @@ let gSubDialog = {
                               .getInterface(Ci.nsIWebNavigation)
                               .QueryInterface(Ci.nsIDocShell)
                               .chromeEventHandler;
-    chromeBrowser.removeEventListener("DOMTitleChanged", gSubDialog.updateTitle, true);
+    chromeBrowser.removeEventListener("DOMTitleChanged", this, true);
   },
 
   updateTitle: function(aEvent) {
@@ -68,6 +72,10 @@ let gSubDialog = {
     if (aClosingCallback) {
       this._closingCallback = aClosingCallback.bind(dialog);
     }
+
+    this._closingEvent = null;
+    this._isClosing = false;
+
     features = features.replace(/,/g, "&");
     let featureParams = new URLSearchParams(features.toLowerCase());
     this._box.setAttribute("resizable", featureParams.has("resizable") &&
@@ -77,6 +85,11 @@ let gSubDialog = {
   },
 
   close: function(aEvent = null) {
+    if (this._isClosing) {
+      return;
+    }
+    this._isClosing = true;
+
     if (this._closingCallback) {
       try {
         this._closingCallback.call(null, aEvent);
@@ -102,8 +115,39 @@ let gSubDialog = {
     }, 0);
   },
 
+  handleEvent: function(aEvent) {
+    switch (aEvent.type) {
+      case "command":
+        this.close(aEvent);
+        break;
+      case "dialogclosing":
+        this._onDialogClosing(aEvent);
+        break;
+      case "DOMTitleChanged":
+        this.updateTitle(aEvent);
+        break;
+      case "DOMFrameContentLoaded":
+        this._onContentLoaded(aEvent);
+        break;
+      case "load":
+        this._onLoad(aEvent);
+        break;
+      case "unload":
+        this._onUnload(aEvent);
+        break;
+      default:
+        break;
+    }
+  },
+
   /* Private methods */
 
+  _onUnload: function(aEvent) {
+    if (aEvent.target.location.href != "about:blank") {
+      this.close(this._closingEvent);
+    }
+  },
+
   _onContentLoaded: function(aEvent) {
     if (aEvent.target != this._frame || aEvent.target.contentWindow.location == "about:blank")
       return;
@@ -115,15 +159,22 @@ let gSubDialog = {
     // Provide the ability for the dialog to know that it is being loaded "in-content".
     this._frame.contentDocument.documentElement.setAttribute("subdialog", "true");
 
+    this._frame.contentWindow.addEventListener("dialogclosing", this);
+
     // Make window.close calls work like dialog closing.
     let oldClose = this._frame.contentWindow.close;
     this._frame.contentWindow.close = function() {
-      var closingEvent = new CustomEvent("dialogclosing", {
-        bubbles: true,
-        detail: { button: null },
-      });
-      gSubDialog._frame.contentWindow.dispatchEvent(closingEvent);
+      var closingEvent = gSubDialog._closingEvent;
+      if (!closingEvent) {
+        closingEvent = new CustomEvent("dialogclosing", {
+          bubbles: true,
+          detail: { button: null },
+        });
 
+        gSubDialog._frame.contentWindow.dispatchEvent(closingEvent);
+      }
+
+      gSubDialog.close(closingEvent);
       oldClose.call(gSubDialog._frame.contentWindow);
     };
 
@@ -132,11 +183,6 @@ let gSubDialog = {
     // the dialog's load event.
     this._overlay.style.visibility = "visible";
     this._overlay.style.opacity = "0.01";
-
-    this._frame.contentWindow.addEventListener("dialogclosing", function closingDialog(aEvent) {
-      gSubDialog._frame.contentWindow.removeEventListener("dialogclosing", closingDialog);
-      gSubDialog.close(aEvent);
-    });
   },
 
   _onLoad: function(aEvent) {
@@ -187,4 +233,9 @@ let gSubDialog = {
     this._frame.focus();
     this._overlay.style.opacity = ""; // XXX: focus hack continued from _onContentLoaded
   },
+
+  _onDialogClosing: function(aEvent) {
+    this._frame.contentWindow.removeEventListener("dialogclosing", this);
+    this._closingEvent = aEvent;
+  }
 };

browser/components/preferences/in-content/tests/browser_subdialogs.js

@@ -76,7 +76,7 @@ let gTests = [{
     dialog.document.documentElement.cancelDialog();
 
     let closingEvent = yield closingPromise;
-    ise(closingEvent.detail.button, "cancel", "closing event should indicate button was 'accept'");
+    ise(closingEvent.detail.button, "cancel", "closing event should indicate button was 'cancel'");
 
     yield deferredClose.promise;
     ise(rv.acceptCount, 0, "return value should NOT have been updated");
@@ -118,6 +118,22 @@ let gTests = [{
     ise(rv.acceptCount, 0, "return value should NOT have been updated");
   },
 },
+{
+  desc: "Check that 'back' navigation will close the dialog",
+  run: function* () {
+    let rv = { acceptCount: 0 };
+    let deferredClose = Promise.defer();
+    let dialogPromise = openAndLoadSubDialog(gDialogURL, null, rv,
+                                             (aEvent) => dialogClosingCallback(deferredClose, aEvent));
+    let dialog = yield dialogPromise;
+
+    info("cancelling the dialog");
+    content.gSubDialog._frame.goBack();
+
+    yield deferredClose.promise;
+    ise(rv.acceptCount, 0, "return value should NOT have been updated");
+  },
+},
 {
   desc: "Hitting escape in the dialog",
   run: function* () {