Bug 1238032 - fix dependent tab close issues by part-reverting bug 305085, r?jaws draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 04 Apr 2016 14:43:37 +0100
changeset 347292 d9d2068466d23d76e9c9126960cba1e5d57d1221
parent 347280 2ffa9007ef203a29052b6db3e1dd63c0349cc70e
child 517591 e5591d678c4781d5489cf559ea9be8706406725e
push id14539
push usergijskruitbosch@gmail.com
push dateMon, 04 Apr 2016 13:51:29 +0000
reviewersjaws
bugs1238032, 305085
milestone48.0a1
Bug 1238032 - fix dependent tab close issues by part-reverting bug 305085, r?jaws MozReview-Commit-ID: IRaytDjRHe6
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_tab_close_dependent_window.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2241,38 +2241,16 @@
         <body>
           <![CDATA[
             if (aTab.closing ||
                 this._windowIsClosing)
               return false;
 
             var browser = this.getBrowserForTab(aTab);
 
-            var closeWindow = false;
-            var newTab = false;
-            if (this.tabs.length - this._removingTabs.length == 1) {
-              closeWindow = aCloseWindowWithLastTab != null ? aCloseWindowWithLastTab :
-                            !window.toolbar.visible ||
-                              Services.prefs.getBoolPref("browser.tabs.closeWindowWithLastTab");
-
-              // Closing the tab and replacing it with a blank one is notably slower
-              // than closing the window right away. If the caller opts in, take
-              // the fast path.
-              if (closeWindow &&
-                  aCloseWindowFastpath &&
-                  this._removingTabs.length == 0) {
-                // This call actually closes the window, unless the user
-                // cancels the operation.  We are finished here in both cases.
-                this._windowIsClosing = window.closeWindow(true, window.warnAboutClosingWindow);
-                return null;
-              }
-
-              newTab = true;
-            }
-
             if (!aTab._pendingPermitUnload && !aAdoptedByTab && !aSkipPermitUnload) {
               // We need to block while calling permitUnload() because it
               // processes the event queue and may lead to another removeTab()
               // call before permitUnload() returns.
               aTab._pendingPermitUnload = true;
               let {permitUnload} = browser.permitUnload();
               delete aTab._pendingPermitUnload;
               // If we were closed during onbeforeunload, we return false now
@@ -2280,16 +2258,45 @@
               // 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;
               }
             }
 
+
+            var closeWindow = false;
+            var newTab = false;
+            if (this.tabs.length - this._removingTabs.length == 1) {
+              closeWindow = aCloseWindowWithLastTab != null ? aCloseWindowWithLastTab :
+                            !window.toolbar.visible ||
+                              Services.prefs.getBoolPref("browser.tabs.closeWindowWithLastTab");
+
+              if (closeWindow) {
+                // We've already called beforeunload on all the relevant tabs if we get here,
+                // so avoid calling it again:
+                window.skipNextCanClose = true;
+              }
+
+              // Closing the tab and replacing it with a blank one is notably slower
+              // than closing the window right away. If the caller opts in, take
+              // the fast path.
+              if (closeWindow &&
+                  aCloseWindowFastpath &&
+                  this._removingTabs.length == 0) {
+                // This call actually closes the window, unless the user
+                // cancels the operation.  We are finished here in both cases.
+                this._windowIsClosing = window.closeWindow(true, window.warnAboutClosingWindow);
+                return null;
+              }
+
+              newTab = true;
+            }
+
             aTab.closing = true;
             this._removingTabs.push(aTab);
             this._visibleTabs = null; // invalidate cache
 
             // Invalidate hovered tab state tracking for this closing tab.
             if (this.tabContainer._hoveredTab == aTab)
               aTab._mouseleave();
 
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -418,16 +418,17 @@ support-files =
   contentSearchUI.js
 [browser_selectpopup.js]
 run-if = e10s
 [browser_selectTabAtIndex.js]
 [browser_ssl_error_reports.js]
 [browser_star_hsts.js]
 [browser_subframe_favicons_not_used.js]
 [browser_syncui.js]
+[browser_tab_close_dependent_window.js]
 [browser_tabDrop.js]
 skip-if = buildapp == 'mulet'
 [browser_tabReorder.js]
 skip-if = buildapp == 'mulet'
 [browser_tabMatchesInAwesomebar.js]
 [browser_tabMatchesInAwesomebar_perwindowpb.js]
 skip-if = os == 'linux' # Bug 1104755
 [browser_tab_detach_restore.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_tab_close_dependent_window.js
@@ -0,0 +1,24 @@
+"use strict";
+
+add_task(function* closing_tab_with_dependents_should_close_window() {
+  info("Opening window");
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+
+  info("Opening tab with data URI");
+  let tab = yield BrowserTestUtils.openNewForegroundTab(win.gBrowser, `data:text/html,<html%20onclick="W=window.open()"><body%20onbeforeunload="W.close()">`);
+  info("Closing original tab in this window.");
+  yield BrowserTestUtils.removeTab(win.gBrowser.tabs[0]);
+  info("Clicking into the window");
+  let depTabOpened = BrowserTestUtils.waitForEvent(win.gBrowser.tabContainer, "TabOpen");
+  yield BrowserTestUtils.synthesizeMouse("html", 0, 0, {}, tab.linkedBrowser);
+
+  let openedTab = (yield depTabOpened).target;
+  info("Got opened tab");
+
+  let windowClosedPromise = BrowserTestUtils.windowClosed(win);
+  yield BrowserTestUtils.removeTab(tab);
+  is(openedTab.linkedBrowser, null, "Opened tab should also have closed");
+  info("If we timeout now, the window failed to close - that shouldn't happen!");
+  yield windowClosedPromise;
+});
+