Bug 1257182 - "Restore All Tabs" can fail when there are pre-existing tabs. r=dao
☠☠ backed out by 9e938575c485 ☠ ☠
authorBlair McBride <bmcbride@mozilla.com>
Tue, 24 May 2016 12:19:31 +1200
changeset 370271 b3835efbf422b78ea753a5072d3e25986b7df180
parent 370270 cb4081774f86e6f121feeb86c923ec54de47441c
child 370272 9e938575c485151e55c05dd7a1f824bdd8c64837
push id19026
push userbobowencode@gmail.com
push dateTue, 24 May 2016 12:34:53 +0000
reviewersdao
bugs1257182
milestone49.0a1
Bug 1257182 - "Restore All Tabs" can fail when there are pre-existing tabs. r=dao MozReview-Commit-ID: HZqflYBJfJy
browser/base/content/browser.js
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_restoreClosedTabs.js
browser/components/sessionstore/SessionStore.jsm
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6299,17 +6299,17 @@ function convertFromUnicode(charset, str
 
 /**
  * Re-open a closed tab.
  * @param aIndex
  *        The index of the tab (via SessionStore.getClosedTabData)
  * @returns a reference to the reopened tab.
  */
 function undoCloseTab(aIndex) {
-  // wallpaper patch to prevent an unnecessary blank tab (bug 343895)
+  // Prevent an unnecessary blank tab left behind
   var blankTabToRemove = null;
   if (gBrowser.tabs.length == 1 && isTabEmpty(gBrowser.selectedTab))
     blankTabToRemove = gBrowser.selectedTab;
 
   var tab = null;
   if (SessionStore.getClosedTabCount(window) > (aIndex || 0)) {
     tab = SessionStore.undoCloseTab(window, aIndex || 0);
 
@@ -6340,16 +6340,21 @@ function undoCloseWindow(aIndex) {
  */
 function isTabEmpty(aTab) {
   if (aTab.hasAttribute("busy"))
     return false;
 
   if (aTab.hasAttribute("customizemode"))
     return false;
 
+  // Tab may be in the early stages of being restored - so even though it's
+  // currently blank, we don't want to treat it as such.
+  if (SessionStore.isTabRestoring(aTab))
+    return false;
+
   let browser = aTab.linkedBrowser;
   if (!isBlankPageURL(browser.currentURI.spec))
     return false;
 
   if (!checkEmptyPageOrigin(browser))
     return false;
 
   if (browser.canGoForward || browser.canGoBack)
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -376,16 +376,17 @@ support-files =
   refresh_meta.sjs
 [browser_relatedTabs.js]
 [browser_remoteTroubleshoot.js]
 support-files =
   test_remoteTroubleshoot.html
 [browser_remoteWebNavigation_postdata.js]
 [browser_removeTabsToTheEnd.js]
 [browser_restore_isAppTab.js]
+[browser_restoreClosedTabs.js]
 [browser_sanitize-passwordDisabledHosts.js]
 [browser_sanitize-sitepermissions.js]
 [browser_sanitize-timespans.js]
 skip-if = buildapp == 'mulet'
 [browser_sanitizeDialog.js]
 skip-if = buildapp == 'mulet'
 [browser_save_link-perwindowpb.js]
 skip-if = buildapp == 'mulet'
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_restoreClosedTabs.js
@@ -0,0 +1,233 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This test opens a new window for each test part, to ensure we have
+// a clean slate each time.
+
+add_task(function*() {
+  info("Test Restore All Tabs, including closing the original single blank tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  let blankTab = gBrowser.selectedTab;
+  info("Opening tabs...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+  let tab2 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab2");
+
+  info("Closing tabs...");
+  gBrowser.removeTab(tab1);
+  gBrowser.removeTab(tab2);
+
+  info("Opening PanelUI...");
+  yield win.PanelUI.show();
+  info("Clicking on History button...");
+  let historyBtn = win.document.getElementById("history-panelmenu");
+  EventUtils.synthesizeMouseAtCenter(historyBtn, {}, win);
+  yield BrowserTestUtils.waitForCondition(() => !win.PanelUI.multiView.hasAttribute("transitioning"));
+
+  let newTabPromises = [
+    BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1"),
+    BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab2"),
+  ];
+  let closeTabPromise = BrowserTestUtils.removeTab(blankTab, {dontRemove: true});
+
+  info("Clicking on Restore All Tabs...");
+  let restoreAllTabsBtn = win.document.getElementById("PanelUI-recentlyClosedTabs").firstChild;
+  EventUtils.synthesizeMouseAtCenter(restoreAllTabsBtn, {}, win);
+  let reopenedTabs = yield Promise.all(newTabPromises);
+  info("Tabs restored");
+  yield closeTabPromise;
+  info("Original blank tab closed");
+
+  Assert.equal(gBrowser.tabs.length, 2, "Expect 2 tabs, the blank tab having been closed");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
+
+
+add_task(function*() {
+  info("Test Restore All Tabs, not closing original tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  info("Loading tab to keep");
+  let originalTab = gBrowser.selectedTab;
+  let originalTabLoad = BrowserTestUtils.browserLoaded(originalTab.linkedBrowser);
+  originalTab.linkedBrowser.loadURI("http://example.com/originalTab");
+  yield originalTabLoad;
+
+  info("Opening tabs...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+  let tab2 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab2");
+
+  info("Closing tabs...");
+  gBrowser.removeTab(tab1);
+  gBrowser.removeTab(tab2);
+
+  info("Opening PanelUI...");
+  yield win.PanelUI.show();
+  info("Clicking on History button...");
+  let historyBtn = win.document.getElementById("history-panelmenu");
+  EventUtils.synthesizeMouseAtCenter(historyBtn, {}, win);
+  yield BrowserTestUtils.waitForCondition(() => !win.PanelUI.multiView.hasAttribute("transitioning"));
+
+  let newTabPromises = [
+    BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1"),
+    BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab2"),
+  ];
+
+  info("Clicking on Restore All Tabs...");
+  let restoreAllTabsBtn = win.document.getElementById("PanelUI-recentlyClosedTabs").firstChild;
+  EventUtils.synthesizeMouseAtCenter(restoreAllTabsBtn, {}, win);
+  let reopenedTabs = yield Promise.all(newTabPromises);
+  info("Tabs restored");
+
+  Assert.equal(gBrowser.tabs.length, 3, "Expect 3 tabs, the original tab was kept open");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
+
+
+add_task(function*() {
+  info("Test undo closing a single tab via PanelUI, including closing the original single blank tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  let blankTab = gBrowser.selectedTab;
+  info("Opening tab...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+
+  info("Closing tab...");
+  gBrowser.removeTab(tab1);
+
+  info("Opening PanelUI...");
+  yield win.PanelUI.show();
+  info("Clicking on History button...");
+  let historyBtn = win.document.getElementById("history-panelmenu");
+  EventUtils.synthesizeMouseAtCenter(historyBtn, {}, win);
+  yield BrowserTestUtils.waitForCondition(() => !win.PanelUI.multiView.hasAttribute("transitioning"));
+
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1");
+  let closeTabPromise = BrowserTestUtils.removeTab(blankTab, {dontRemove: true});
+
+  info("Clicking on item to restore tab...");
+  let restoreTabBtn = win.document.getElementById("PanelUI-recentlyClosedTabs").firstChild.nextSibling;
+  EventUtils.synthesizeMouseAtCenter(restoreTabBtn, {}, win);
+  let reopenedTab = yield newTabPromise;
+  info("Tab restored");
+  yield closeTabPromise;
+  info("Original blank tab closed");
+
+  Assert.equal(gBrowser.tabs.length, 1, "Expect 1 tab, the blank tab having been closed");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
+
+
+add_task(function*() {
+  info("Test undo closing a single tab via PanelUI, not closing original tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  info("Loading tab to keep");
+  let originalTab = gBrowser.selectedTab;
+  let originalTabLoad = BrowserTestUtils.browserLoaded(originalTab.linkedBrowser);
+  originalTab.linkedBrowser.loadURI("http://example.com/originalTab");
+  yield originalTabLoad;
+
+  info("Opening tab...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+
+  info("Closing tab...");
+  gBrowser.removeTab(tab1);
+
+  info("Opening PanelUI...");
+  yield win.PanelUI.show();
+  info("Clicking on History button...");
+  let historyBtn = win.document.getElementById("history-panelmenu");
+  EventUtils.synthesizeMouseAtCenter(historyBtn, {}, win);
+  yield BrowserTestUtils.waitForCondition(() => !win.PanelUI.multiView.hasAttribute("transitioning"));
+
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1");
+
+  info("Clicking on item to restore tab...");
+  let restoreTabBtn = win.document.getElementById("PanelUI-recentlyClosedTabs").firstChild.nextSibling;
+  EventUtils.synthesizeMouseAtCenter(restoreTabBtn, {}, win);
+  let reopenedTab = yield newTabPromise;
+  info("Tab restored");
+
+  Assert.equal(gBrowser.tabs.length, 2, "Expect 2 tabs, the original tab was kept open");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
+
+
+add_task(function*() {
+  info("Test undo closing a single tab via shortcut, including closing the original single blank tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  let blankTab = gBrowser.selectedTab;
+  info("Opening tab...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+
+  info("Closing tab...");
+  yield BrowserTestUtils.removeTab(tab1);
+
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1");
+  let closeTabPromise = BrowserTestUtils.removeTab(blankTab, {dontRemove: true});
+
+  info("Synthesizing key shortcut to restore tab...");
+  EventUtils.synthesizeKey("t", {accelKey: true, shiftKey: true}, win);
+  let reopenedTab = yield newTabPromise;
+  info("Tab restored");
+  yield closeTabPromise;
+  info("Original blank tab closed");
+
+  Assert.equal(gBrowser.tabs.length, 1, "Expect 1 tab, the blank tab having been closed");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
+
+
+
+add_task(function*() {
+  info("Test undo closing a single tab via shortcut, not closing original tab");
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  let gBrowser = win.gBrowser;
+
+  info("Loading tab to keep");
+  let originalTab = gBrowser.selectedTab;
+  let originalTabLoad = BrowserTestUtils.browserLoaded(originalTab.linkedBrowser);
+  originalTab.linkedBrowser.loadURI("http://example.com/originalTab");
+  yield originalTabLoad;
+
+  info("Opening tab...");
+  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/tab1");
+
+  info("Closing tab...");
+  yield BrowserTestUtils.removeTab(tab1);
+
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/tab1");
+
+  info("Synthesizing key shortcut to restore tab...");
+  EventUtils.synthesizeKey("t", {accelKey: true, shiftKey: true}, win);
+  let reopenedTab = yield newTabPromise;
+  info("Tab restored");
+
+  Assert.equal(gBrowser.tabs.length, 2, "Expect 2 tabs, the original tab was kept open");
+
+  // Cleanup
+  yield BrowserTestUtils.closeWindow(win);
+});
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -325,16 +325,26 @@ this.SessionStore = {
 
   navigateAndRestore(tab, loadArguments, historyIndex) {
     return SessionStoreInternal.navigateAndRestore(tab, loadArguments, historyIndex);
   },
 
   getSessionHistory(tab, updatedCallback) {
     return SessionStoreInternal.getSessionHistory(tab, updatedCallback);
   },
+  
+  /**
+   * Determines whether a given tab is currently in the process of being
+   * restored.
+   * @param  {<xul:tab>} tab   Tab to check
+   * @return {Boolean}         True if currently restoring
+   */
+  isTabRestoring(tab) {
+    return !!tab.linkedBrowser.__SS_restoreState;
+  },
 
   /**
    * Determines whether the passed version number is compatible with
    * the current version number of the SessionStore.
    *
    * @param version The format and version of the file, as an array, e.g.
    * ["sessionrestore", 1]
    */
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -251,26 +251,26 @@ this.BrowserTestUtils = {
    *        A string URL to look for in the new tab. If null, allows any non-blank URL.
    *
    * @return {Promise}
    * @resolves With the {xul:tab} when a tab is opened and its location changes to the given URL.
    */
   waitForNewTab(tabbrowser, url) {
     return new Promise((resolve, reject) => {
       tabbrowser.tabContainer.addEventListener("TabOpen", function onTabOpen(openEvent) {
-        tabbrowser.tabContainer.removeEventListener("TabOpen", onTabOpen);
 
         let progressListener = {
           onLocationChange(aBrowser) {
             if (aBrowser != openEvent.target.linkedBrowser ||
                 (url && aBrowser.currentURI.spec != url) ||
                 (!url && aBrowser.currentURI.spec == "about:blank")) {
               return;
             }
 
+            tabbrowser.tabContainer.removeEventListener("TabOpen", onTabOpen);
             tabbrowser.removeTabsProgressListener(progressListener);
             resolve(openEvent.target);
           },
         };
         tabbrowser.addTabsProgressListener(progressListener);
 
       });
     });