Backed out changeset 3d607fcf4dee (bug 887515) for causing failures in toolbar-context-menu-undo-close-tab
authorMihai Alexandru Michis <malexandru@mozilla.com>
Sat, 30 May 2020 09:25:30 +0300
changeset 533104 cf0c39ed1f707f1e6b403298a50153441b4a9355
parent 533103 3d607fcf4deef85cd48f6cd7d0507b62244a0e5c
child 533105 8aaca63ec5c6dd73365ba31d1972771cb847d5bc
push id37462
push usermalexandru@mozilla.com
push dateSat, 30 May 2020 09:46:43 +0000
treeherdermozilla-central@8aaca63ec5c6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs887515
milestone78.0a1
backs out3d607fcf4deef85cd48f6cd7d0507b62244a0e5c
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 3d607fcf4dee (bug 887515) for causing failures in toolbar-context-menu-undo-close-tab CLOSED TREE
browser/base/content/browser-allTabsMenu.inc.xhtml
browser/base/content/browser-sets.inc
browser/base/content/browser.js
browser/base/content/browser.xhtml
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_undo_close_tabs.js
browser/components/sessionstore/SessionStore.jsm
browser/locales/en-US/browser/allTabsMenu.ftl
browser/locales/en-US/browser/tabContextMenu.ftl
browser/locales/en-US/browser/toolbarContextMenu.ftl
--- a/browser/base/content/browser-allTabsMenu.inc.xhtml
+++ b/browser/base/content/browser-allTabsMenu.inc.xhtml
@@ -2,19 +2,19 @@
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <html:template id="allTabsMenu-container">
   <panelview id="allTabsMenu-allTabsView" class="PanelUI-subView">
     <vbox class="panel-subview-body">
       <toolbarbutton id="allTabsMenu-undoCloseTab"
                      class="subviewbutton subviewbutton-iconic"
-                     data-l10n-id="all-tabs-menu-undo-close-tabs"
+                     data-l10n-id="all-tabs-menu-undo-close-tab"
                      key="key_undoCloseTab"
-                     observes="History:UndoCloseTab"/>
+                     command="History:UndoCloseTab"/>
       <toolbarbutton id="allTabsMenu-searchTabs"
                      class="subviewbutton subviewbutton-iconic"
                      oncommand="gTabsPanel.searchTabs();"
                      data-l10n-id="all-tabs-menu-search-tabs"/>
       <toolbarbutton id="allTabsMenu-containerTabsButton"
                      class="subviewbutton subviewbutton-nav"
                      closemenu="none"
                      oncommand="PanelUI.showSubView('allTabsMenu-containerTabsView', this);"
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -94,17 +94,17 @@
 #ifdef NIGHTLY_BUILD
     <command id="Tools:FissionWindow"
       oncommand="OpenBrowserWindow({fission: true});"
       hidden="true"/>
     <command id="Tools:NonFissionWindow"
       oncommand="OpenBrowserWindow({fission: false});"
       hidden="true"/>
 #endif
-    <command id="History:UndoCloseTab" oncommand="undoCloseTab();" data-l10n-args='{"tabCount": 1}'/>
+    <command id="History:UndoCloseTab" oncommand="undoCloseTab();"/>
     <command id="History:UndoCloseWindow" oncommand="undoCloseWindow();"/>
 
     <command id="wrCaptureCmd" oncommand="gGfxUtils.webrenderCapture();" disabled="true"/>
     <command id="wrToggleCaptureSequenceCmd" oncommand="gGfxUtils.toggleWebrenderCaptureSequence();" disabled="true"/>
 #ifdef NIGHTLY_BUILD
     <command id="windowRecordingCmd" oncommand="gGfxUtils.toggleWindowRecording();"/>
 #endif
 #ifdef XP_MACOSX
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -8034,37 +8034,29 @@ function AddKeywordForSearchField() {
 /**
  * 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)
-  let blankTabToRemove = null;
+  var blankTabToRemove = null;
   if (gBrowser.tabs.length == 1 && gBrowser.selectedTab.isEmpty) {
     blankTabToRemove = gBrowser.selectedTab;
   }
 
-  let tab = null;
-  // aIndex is undefined if the function is called without a specific tab to restore.
-  let tabsToRemove =
-    aIndex !== undefined
-      ? [aIndex]
-      : new Array(SessionStore.getLastClosedTabCount(window)).fill(0);
-  for (let index of tabsToRemove) {
-    if (SessionStore.getClosedTabCount(window) > index) {
-      tab = SessionStore.undoCloseTab(window, index);
-
-      if (blankTabToRemove) {
-        gBrowser.removeTab(blankTabToRemove);
-      }
-    }
-  }
-  SessionStore.setLastClosedTabCount(window, 1);
+  var tab = null;
+  if (SessionStore.getClosedTabCount(window) > (aIndex || 0)) {
+    tab = SessionStore.undoCloseTab(window, aIndex || 0);
+
+    if (blankTabToRemove) {
+      gBrowser.removeTab(blankTabToRemove);
+    }
+  }
 
   return tab;
 }
 
 /**
  * Re-open a closed window.
  * @param aIndex
  *        The index of the window (via SessionStore.getClosedWindowData)
--- a/browser/base/content/browser.xhtml
+++ b/browser/base/content/browser.xhtml
@@ -180,28 +180,22 @@
         </menupopup>
       </menu>
       <menu id="context_sendTabToDevice"
             class="sync-ui-item">
         <menupopup id="context_sendTabToDevicePopupMenu"
                    onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, TabContextMenu.contextTab.linkedBrowser.currentURI.spec, TabContextMenu.contextTab.linkedBrowser.contentTitle, TabContextMenu.contextTab.multiselected);"/>
       </menu>
       <menuseparator/>
-      <menu id="context_closeTabOptions"
-            data-lazy-l10n-id="tab-context-close-multiple-tabs">
-        <menupopup id="closeTabOptions">
-          <menuitem id="context_closeTabsToTheEnd" data-lazy-l10n-id="close-tabs-to-the-end"
-                    oncommand="gBrowser.removeTabsToTheEndFrom(TabContextMenu.contextTab, {animate: true});"/>
-          <menuitem id="context_closeOtherTabs" data-lazy-l10n-id="close-other-tabs"
-                    oncommand="gBrowser.removeAllTabsBut(TabContextMenu.contextTab);"/>
-        </menupopup>
-      </menu>
+      <menuitem id="context_closeTabsToTheEnd" data-lazy-l10n-id="close-tabs-to-the-end"
+                oncommand="gBrowser.removeTabsToTheEndFrom(TabContextMenu.contextTab, {animate: true});"/>
+      <menuitem id="context_closeOtherTabs" data-lazy-l10n-id="close-other-tabs"
+                oncommand="gBrowser.removeAllTabsBut(TabContextMenu.contextTab);"/>
       <menuitem id="context_undoCloseTab"
-                data-lazy-l10n-id="tab-context-undo-close-tabs"
-                data-l10n-args='{"tabCount": 1}'
+                data-lazy-l10n-id="undo-close-tab"
                 observes="History:UndoCloseTab"/>
       <menuitem id="context_closeTab" data-lazy-l10n-id="close-tab"
                 oncommand="gBrowser.removeTab(TabContextMenu.contextTab, { animate: true });"/>
       <menuitem id="context_closeSelectedTabs" data-lazy-l10n-id="close-tabs"
                 hidden="true"
                 oncommand="gBrowser.removeMultiSelectedTabs();"/>
     </menupopup>
 
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2540,23 +2540,16 @@
       // all callers of addTab that pass a params object need to pass
       // a valid triggeringPrincipal.
       if (!triggeringPrincipal) {
         throw new Error(
           "Required argument triggeringPrincipal missing within addTab"
         );
       }
 
-      // Don't use document.l10n.setAttributes because the FTL file is loaded
-      // lazily and we won't be able to resolve the string.
-      document
-        .getElementById("History:UndoCloseTab")
-        .setAttribute("data-l10n-args", JSON.stringify({ tabCount: 1 }));
-      SessionStore.setLastClosedTabCount(window, 1);
-
       // if we're adding tabs, we're past interrupt mode, ditch the owner
       if (this.selectedTab.owner) {
         this.selectedTab.owner = null;
       }
 
       // Find the tab that opened this one, if any. This is used for
       // determining positioning, and inherited attributes such as the
       // user context ID.
@@ -3296,17 +3289,16 @@
       if (
         this.tabs.length == tabs.length &&
         Services.prefs.getBoolPref("browser.tabs.closeWindowWithLastTab")
       ) {
         window.closeWindow(true, window.warnAboutClosingWindow);
         return;
       }
 
-      let initialTabCount = tabs.length;
       this._clearMultiSelectionLocked = true;
 
       // Guarantee that _clearMultiSelectionLocked lock gets released.
       try {
         let tabsWithBeforeUnload = [];
         let lastToClose;
         let aParams = { animate: true, prewarmed: true };
         for (let tab of tabs) {
@@ -3332,27 +3324,16 @@
           this.removeTab(lastToClose, aParams);
         }
       } catch (e) {
         Cu.reportError(e);
       }
 
       this._clearMultiSelectionLocked = false;
       this.avoidSingleSelectedTab();
-      let closedTabsCount =
-        initialTabCount - tabs.filter(t => !t.closing).length;
-      // Don't use document.l10n.setAttributes because the FTL file is loaded
-      // lazily and we won't be able to resolve the string.
-      document
-        .getElementById("History:UndoCloseTab")
-        .setAttribute(
-          "data-l10n-args",
-          JSON.stringify({ tabCount: closedTabsCount })
-        );
-      SessionStore.setLastClosedTabCount(window, closedTabsCount);
     },
 
     removeCurrentTab(aParams) {
       this.removeTab(this.selectedTab, aParams);
     },
 
     removeTab(
       aTab,
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -93,14 +93,13 @@ skip-if = (os == 'linux' && bits == 64) 
 [browser_tabReorder_overflow.js]
 [browser_tabReorder.js]
 [browser_tabSpinnerProbe.js]
 skip-if = !e10s # Tab spinner is e10s only.
 [browser_tabSuccessors.js]
 [browser_tabSwitchPrintPreview.js]
 skip-if = os == 'mac'
 [browser_tabswitch_updatecommands.js]
-[browser_undo_close_tabs.js]
 [browser_viewsource_of_data_URI_in_file_process.js]
 [browser_visibleTabs_bookmarkAllTabs.js]
 [browser_visibleTabs_contextMenu.js]
 [browser_tabswitch_window_focus.js]
 support-files = open_window_in_new_tab.html
deleted file mode 100644
--- a/browser/base/content/test/tabs/browser_undo_close_tabs.js
+++ /dev/null
@@ -1,102 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-const PREF_WARN_ON_CLOSE = "browser.tabs.warnOnCloseOtherTabs";
-
-add_task(async function setPref() {
-  await SpecialPowers.pushPrefEnv({
-    set: [[PREF_WARN_ON_CLOSE, false]],
-  });
-});
-
-add_task(async function withMultiSelectedTabs() {
-  let initialTab = gBrowser.selectedTab;
-  let tab1 = await addTab("https://example.com/1");
-  let tab2 = await addTab("https://example.com/2");
-  let tab3 = await addTab("https://example.com/3");
-  let tab4 = await addTab("https://example.com/4");
-
-  is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
-
-  gBrowser.selectedTab = tab2;
-  await triggerClickOn(tab4, { shiftKey: true });
-
-  ok(!initialTab.multiselected, "InitialTab is not multiselected");
-  ok(!tab1.multiselected, "Tab1 is not multiselected");
-  ok(tab2.multiselected, "Tab2 is multiselected");
-  ok(tab3.multiselected, "Tab3 is multiselected");
-  ok(tab4.multiselected, "Tab4 is multiselected");
-  is(gBrowser.multiSelectedTabsCount, 3, "Two multiselected tabs");
-
-  gBrowser.removeMultiSelectedTabs();
-  await TestUtils.waitForCondition(
-    () => gBrowser.tabs.length == 2,
-    "wait for the multiselected tabs to close"
-  );
-  is(
-    SessionStore.getLastClosedTabCount(window),
-    3,
-    "SessionStore should know how many tabs were just closed"
-  );
-
-  undoCloseTab();
-  await TestUtils.waitForCondition(
-    () => gBrowser.tabs.length == 5,
-    "wait for the tabs to reopen"
-  );
-  info("waiting for the browsers to finish loading");
-  // Check that the tabs are restored in the correct order
-  for (let tabId of [2, 3, 4]) {
-    let browser = gBrowser.tabs[tabId].linkedBrowser;
-    await ContentTask.spawn(browser, tabId, async aTabId => {
-      await ContentTaskUtils.waitForCondition(() => {
-        return (
-          content?.document?.readyState == "complete" &&
-          content?.document?.location.href == "https://example.com/" + aTabId
-        );
-      }, "waiting for tab " + aTabId + " to load");
-    });
-  }
-
-  gBrowser.removeAllTabsBut(initialTab);
-});
-
-add_task(async function withCloseTabsToTheRight() {
-  let initialTab = gBrowser.selectedTab;
-  let tab1 = await addTab("https://example.com/1");
-  await addTab("https://example.com/2");
-  await addTab("https://example.com/3");
-  await addTab("https://example.com/4");
-
-  gBrowser.removeTabsToTheEndFrom(tab1);
-  await TestUtils.waitForCondition(
-    () => gBrowser.tabs.length == 2,
-    "wait for the multiselected tabs to close"
-  );
-  is(
-    SessionStore.getLastClosedTabCount(window),
-    3,
-    "SessionStore should know how many tabs were just closed"
-  );
-
-  undoCloseTab();
-  await TestUtils.waitForCondition(
-    () => gBrowser.tabs.length == 5,
-    "wait for the tabs to reopen"
-  );
-  info("waiting for the browsers to finish loading");
-  // Check that the tabs are restored in the correct order
-  for (let tabId of [2, 3, 4]) {
-    let browser = gBrowser.tabs[tabId].linkedBrowser;
-    ContentTask.spawn(browser, tabId, async aTabId => {
-      await ContentTaskUtils.waitForCondition(() => {
-        return (
-          content?.document?.readyState == "complete" &&
-          content?.document?.location.href == "https://example.com/" + aTabId
-        );
-      }, "waiting for tab " + aTabId + " to load");
-    });
-  }
-
-  gBrowser.removeAllTabsBut(initialTab);
-});
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -323,23 +323,16 @@ var SessionStore = {
       aWindow,
       aTab,
       aDelta,
       aRestoreImmediately,
       aOptions
     );
   },
 
-  getLastClosedTabCount(aWindow) {
-    return SessionStoreInternal.getLastClosedTabCount(aWindow);
-  },
-  setLastClosedTabCount(aWindow, aNumber) {
-    return SessionStoreInternal.setLastClosedTabCount(aWindow, aNumber);
-  },
-
   getClosedTabCount: function ss_getClosedTabCount(aWindow) {
     return SessionStoreInternal.getClosedTabCount(aWindow);
   },
 
   getClosedTabData: function ss_getClosedTabData(aWindow, aAsString = true) {
     return SessionStoreInternal.getClosedTabData(aWindow, aAsString);
   },
 
@@ -725,17 +718,16 @@ var SessionStoreInternal = {
 
     TelemetryTimestamps.add("sessionRestoreInitialized");
     OBSERVING.forEach(function(aTopic) {
       Services.obs.addObserver(this, aTopic, true);
     }, this);
 
     this._initPrefs();
     this._initialized = true;
-    this._closedTabCache = new WeakMap();
 
     Telemetry.getHistogramById("FX_SESSION_RESTORE_PRIVACY_LEVEL").add(
       Services.prefs.getIntPref("browser.sessionstore.privacy_level")
     );
   },
 
   /**
    * Initialize the session using the state provided by SessionStartup
@@ -3178,38 +3170,16 @@ var SessionStoreInternal = {
       this.restoreTab(newTab, tabState, {
         restoreImmediately: aRestoreImmediately,
       });
     });
 
     return newTab;
   },
 
-  getLastClosedTabCount(aWindow) {
-    if ("__SSi" in aWindow) {
-      // Blank tabs cannot be undo-closed, so the number returned by
-      // the ClosedTabCache can be greater than the return value of
-      // getClosedTabCount. We won't restore blank tabs, so we return
-      // the minimum of these two values.
-      return Math.min(
-        this._closedTabCache.get(aWindow) || 1,
-        this.getClosedTabCount(aWindow)
-      );
-    }
-
-    throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
-  },
-  setLastClosedTabCount(aWindow, aNumber) {
-    if ("__SSi" in aWindow) {
-      return this._closedTabCache.set(aWindow, aNumber);
-    }
-
-    throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
-  },
-
   getClosedTabCount: function ssi_getClosedTabCount(aWindow) {
     if ("__SSi" in aWindow) {
       return this._windows[aWindow.__SSi]._closedTabs.length;
     }
 
     if (!DyingWindowCache.has(aWindow)) {
       throw Components.Exception(
         "Window is not tracked",
--- a/browser/locales/en-US/browser/allTabsMenu.ftl
+++ b/browser/locales/en-US/browser/allTabsMenu.ftl
@@ -1,18 +1,14 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-all-tabs-menu-undo-close-tabs =
-  .label =
-      { $tabCount ->
-          [1] Undo Close Tab
-         *[other] Undo Close Tabs
-      }
+all-tabs-menu-undo-close-tab =
+  .label = Undo Close Tab
 
 # "Search" is a verb, as in "Search through tabs".
 all-tabs-menu-search-tabs =
   .label = Search Tabs
 
 all-tabs-menu-new-user-context =
   .label = New Container Tab
 
--- a/browser/locales/en-US/browser/tabContextMenu.ftl
+++ b/browser/locales/en-US/browser/tabContextMenu.ftl
@@ -48,25 +48,18 @@ move-to-start =
     .label = Move to Start
     .accesskey = S
 move-to-end =
     .label = Move to End
     .accesskey = E
 move-to-new-window =
     .label = Move to New Window
     .accesskey = W
-tab-context-close-multiple-tabs =
-    .label = Close Multiple Tabs
-    .accesskey = M
-tab-context-undo-close-tabs =
-    .label =
-        { $tabCount ->
-            [1] Undo Close Tab
-           *[other] Undo Close Tabs
-        }
+undo-close-tab =
+    .label = Undo Close Tab
     .accesskey = U
 close-tab =
     .label = Close Tab
     .accesskey = c
 close-tabs =
     .label = Close Tabs
     .accesskey = S
 move-tabs =
--- a/browser/locales/en-US/browser/toolbarContextMenu.ftl
+++ b/browser/locales/en-US/browser/toolbarContextMenu.ftl
@@ -12,22 +12,18 @@ toolbar-context-menu-bookmark-selected-t
     .label = Bookmark Selected Tab…
     .accesskey = T
 toolbar-context-menu-bookmark-selected-tabs =
     .label = Bookmark Selected Tabs…
     .accesskey = T
 toolbar-context-menu-select-all-tabs =
     .label = Select All Tabs
     .accesskey = S
-toolbar-context-menu-undo-close-tabs =
-    .label =
-        { $tabCount ->
-            [1] Undo Close Tab
-           *[other] Undo Close Tabs
-        }
+toolbar-context-menu-undo-close-tab =
+    .label = Undo Close Tab
     .accesskey = U
 
 toolbar-context-menu-manage-extension =
     .label = Manage Extension
     .accesskey = E
 toolbar-context-menu-remove-extension =
     .label = Remove Extension
     .accesskey = v