Bug 1458022 - Implement ability to close a selection of tabs. r?jaws draft multi_close
authorlayely <ablayelyfondou@gmail.com>
Mon, 21 May 2018 04:16:01 +0000
branchmulti_close
changeset 799166 7dd815d37aae1676f828165afecc8c3f88777113
parent 798084 b75acf9652937ce79a9bf02de843c100db0e5ec7
push id110951
push userbmo:ablayelyfondou@gmail.com
push dateThu, 24 May 2018 03:38:30 +0000
reviewersjaws
bugs1458022
milestone62.0a1
Bug 1458022 - Implement ability to close a selection of tabs. r?jaws MozReview-Commit-ID: 4yjajKnONuK
browser/base/content/browser.js
browser/base/content/browser.xul
browser/base/content/tabbrowser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_multiselect_tabs_close.js
browser/locales/en-US/chrome/browser/browser.dtd
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7826,16 +7826,21 @@ var TabContextMenu = {
 
     // Disable "Close other Tabs" if there are no unpinned tabs.
     let unpinnedTabsToClose = gBrowser.visibleTabs.length - gBrowser._numPinnedTabs;
     if (!this.contextTab.pinned) {
       unpinnedTabsToClose--;
     }
     document.getElementById("context_closeOtherTabs").disabled = unpinnedTabsToClose < 1;
 
+    // Only one of close_tab/close_selected_tabs should be visible
+    let hasMultiSelectedTabs = !!gBrowser.multiSelectedTabsCount;
+    document.getElementById("context_closeTab").hidden = hasMultiSelectedTabs;
+    document.getElementById("context_closeSelectedTabs").hidden = !hasMultiSelectedTabs;
+
     // Hide "Bookmark All Tabs" for a pinned tab.  Update its state if visible.
     let bookmarkAllTabs = document.getElementById("context_bookmarkAllTabs");
     bookmarkAllTabs.hidden = this.contextTab.pinned;
     if (!bookmarkAllTabs.hidden)
       PlacesCommandHook.updateBookmarkAllTabsCommand();
 
     // Adjust the state of the toggle mute menu item.
     let toggleMute = document.getElementById("context_toggleMuteTab");
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -135,16 +135,19 @@
                 oncommand="gBrowser.removeAllTabsBut(TabContextMenu.contextTab);"/>
       <menuseparator/>
       <menuitem id="context_undoCloseTab"
                 label="&undoCloseTab.label;"
                 accesskey="&undoCloseTab.accesskey;"
                 observes="History:UndoCloseTab"/>
       <menuitem id="context_closeTab" label="&closeTab.label;" accesskey="&closeTab.accesskey;"
                 oncommand="gBrowser.removeTab(TabContextMenu.contextTab, { animate: true });"/>
+      <menuitem id="context_closeSelectedTabs" label="&closeSelectedTabs.label;"
+                hidden="true" accesskey="&closeSelectedTabs.accesskey;"
+                oncommand="gBrowser.removeMultiSelectedTabs();"/>
     </menupopup>
 
     <!-- bug 415444/582485: event.stopPropagation is here for the cloned version
          of this menupopup -->
     <menupopup id="backForwardMenu"
                onpopupshowing="return FillHistoryMenu(event.target);"
                oncommand="gotoHistoryIndex(event); event.stopPropagation();"
                onclick="checkForMiddleClick(this, event);"/>
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -64,17 +64,17 @@ window._gBrowser = {
 
     this._setupEventListeners();
   },
 
   ownerGlobal: window,
 
   ownerDocument: document,
 
-  closingTabsEnum: { ALL: 0, OTHER: 1, TO_END: 2 },
+  closingTabsEnum: { ALL: 0, OTHER: 1, TO_END: 2, MULTI_SELECTED: 3 },
 
   _visibleTabs: null,
 
   _lastRelatedTabMap: new WeakMap(),
 
   mProgressListeners: [],
 
   mTabsProgressListeners: [],
@@ -2534,16 +2534,19 @@ window._gBrowser = {
         tabsToClose = this.visibleTabs.length - 1 - gBrowser._numPinnedTabs;
         break;
       case this.closingTabsEnum.TO_END:
         if (!aTab)
           throw new Error("Required argument missing: aTab");
 
         tabsToClose = this.getTabsToTheEndFrom(aTab).length;
         break;
+      case this.closingTabsEnum.MULTI_SELECTED:
+        tabsToClose = this.multiSelectedTabsCount;
+        break;
       default:
         throw new Error("Invalid argument: " + aCloseTabs);
     }
 
     if (tabsToClose <= 1)
       return true;
 
     const pref = aCloseTabs == this.closingTabsEnum.ALL ?
@@ -2593,60 +2596,64 @@ window._gBrowser = {
       if (tabs[i] == aTab || tabs[i].pinned) {
         break;
       }
       tabsToEnd.push(tabs[i]);
     }
     return tabsToEnd;
   },
 
-  removeTabsToTheEndFrom(aTab, aParams) {
+  removeTabsToTheEndFrom(aTab) {
     if (!this.warnAboutClosingTabs(this.closingTabsEnum.TO_END, aTab))
       return;
 
-    let removeTab = tab => {
-      // Avoid changing the selected browser several times.
-      if (tab.selected)
-        this.selectedTab = aTab;
-
-      this.removeTab(tab, aParams);
-    };
-
     let tabs = this.getTabsToTheEndFrom(aTab);
-    let tabsWithBeforeUnload = [];
-    for (let i = tabs.length - 1; i >= 0; --i) {
-      let tab = tabs[i];
-      if (this._hasBeforeUnload(tab))
-        tabsWithBeforeUnload.push(tab);
-      else
-        removeTab(tab);
-    }
-    tabsWithBeforeUnload.forEach(removeTab);
+    this.removeCollectionOfTabs(tabs);
   },
 
   removeAllTabsBut(aTab) {
     if (!this.warnAboutClosingTabs(this.closingTabsEnum.OTHER)) {
       return;
     }
 
-    let tabs = this.visibleTabs.reverse();
+    let tabs = this.visibleTabs.filter(tab => tab != aTab && !tab.pinned);
     this.selectedTab = aTab;
-
+    this.removeCollectionOfTabs(tabs);
+  },
+
+  removeMultiSelectedTabs() {
+    if (!this.warnAboutClosingTabs(this.closingTabsEnum.MULTI_SELECTED)) {
+      return;
+    }
+
+    let selectedTabs = ChromeUtils.nondeterministicGetWeakMapKeys(this._multiSelectedTabsMap)
+                                    .filter(tab => tab.isConnected);
+    this.removeCollectionOfTabs(selectedTabs);
+  },
+
+  removeCollectionOfTabs(tabs) {
     let tabsWithBeforeUnload = [];
-    for (let i = tabs.length - 1; i >= 0; --i) {
-      let tab = tabs[i];
-      if (tab != aTab && !tab.pinned) {
-        if (this._hasBeforeUnload(tab))
-          tabsWithBeforeUnload.push(tab);
-        else
-          this.removeTab(tab, { animate: true });
-      }
+    let lastToClose;
+    let aParams = {animation: true};
+    for (let tab of tabs) {
+      if (tab.selected)
+        lastToClose = tab;
+      else if (this._hasBeforeUnload(tab))
+        tabsWithBeforeUnload.push(tab);
+      else
+        this.removeTab(tab, aParams);
     }
     for (let tab of tabsWithBeforeUnload) {
-      this.removeTab(tab, { animate: true });
+      this.removeTab(tab, aParams);
+    }
+
+    // Avoid changing the selected browser several times by removing it,
+    // if appropriate, lastly.
+    if (lastToClose) {
+      this.removeTab(lastToClose, aParams);
     }
   },
 
   removeCurrentTab(aParams) {
     this.removeTab(this.selectedTab, aParams);
   },
 
   removeTab(aTab, aParams) {
@@ -3632,28 +3639,25 @@ window._gBrowser = {
   addRangeToMultiSelectedTabs(aTab1, aTab2) {
     // Let's avoid going through all the heavy process below when the same
     // tab is given as params.
     if (aTab1 == aTab2) {
       this.addToMultiSelectedTabs(aTab1);
       return;
     }
 
-    const tabs = [...this.tabs];
+    const tabs = this._visibleTabs;
     const indexOfTab1 = tabs.indexOf(aTab1);
     const indexOfTab2 = tabs.indexOf(aTab2);
 
     const [lowerIndex, higherIndex] = indexOfTab1 < indexOfTab2 ?
       [indexOfTab1, indexOfTab2] : [indexOfTab2, indexOfTab1];
 
     for (let i = lowerIndex; i <= higherIndex; i++) {
-      let tab = tabs[i];
-      if (!tab.hidden) {
-        this.addToMultiSelectedTabs(tab);
-      }
+      this.addToMultiSelectedTabs(tabs[i]);
     }
   },
 
   removeFromMultiSelectedTabs(aTab) {
     if (!aTab.multiselected) {
       return;
     }
     aTab.removeAttribute("multiselected");
@@ -3667,17 +3671,17 @@ window._gBrowser = {
         tab.removeAttribute("multiselected");
       }
     }
     this._multiSelectedTabsMap = new WeakMap();
   },
 
   get multiSelectedTabsCount() {
     return ChromeUtils.nondeterministicGetWeakMapKeys(this._multiSelectedTabsMap)
-      .filter(tab => tab.isConnected)
+      .filter(tab => tab.isConnected && !tab.closing)
       .length;
   },
 
   get lastMultiSelectedTab() {
     let tab = this._lastMultiSelectedTabRef ? this._lastMultiSelectedTabRef.get() : null;
     if (tab && tab.isConnected && this._multiSelectedTabsMap.has(tab)) {
       return tab;
     }
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1993,33 +1993,39 @@
             if (this.multiselected) {
               gBrowser.removeFromMultiSelectedTabs(this);
             } else {
               gBrowser.addToMultiSelectedTabs(this);
               gBrowser.lastMultiSelectedTab = this;
             }
             return;
           }
-          if (gBrowser.multiSelectedTabsCount > 0) {
+
+          const overCloseButton = event.originalTarget.getAttribute("anonid") == "close-button";
+          if (gBrowser.multiSelectedTabsCount > 0 && !overCloseButton) {
             // Tabs were previously multi-selected and user clicks on a tab
             // without holding Ctrl/Cmd Key
             gBrowser.clearMultiSelectedTabs();
           }
         }
 
         if (this._overPlayingIcon) {
           this.toggleMuteAudio();
           return;
         }
 
         if (event.originalTarget.getAttribute("anonid") == "close-button") {
-          gBrowser.removeTab(this, {
-            animate: true,
-            byMouse: event.mozInputSource == MouseEvent.MOZ_SOURCE_MOUSE,
-          });
+          if (this.multiselected) {
+            gBrowser.removeMultiSelectedTabs();
+          } else {
+            gBrowser.removeTab(this, {
+              animate: true,
+              byMouse: event.mozInputSource == MouseEvent.MOZ_SOURCE_MOUSE,
+            });
+          }
           // This enables double-click protection for the tab container
           // (see tabbrowser-tabs 'click' handler).
           gBrowser.tabContainer._blockDblClick = true;
         }
       ]]></handler>
 
       <handler event="dblclick" button="0" phase="capturing"><![CDATA[
         // for the one-close-button case
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -39,8 +39,9 @@ skip-if = (debug && os == 'mac') || (deb
 [browser_tabswitch_updatecommands.js]
 [browser_viewsource_of_data_URI_in_file_process.js]
 [browser_visibleTabs_bookmarkAllTabs.js]
 [browser_visibleTabs_contextMenu.js]
 [browser_open_newtab_start_observer_notification.js]
 [browser_bug_1387976_restore_lazy_tab_browser_muted_state.js]
 [browser_multiselect_tabs_using_Ctrl.js]
 [browser_multiselect_tabs_using_Shift.js]
+[browser_multiselect_tabs_close.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_multiselect_tabs_close.js
@@ -0,0 +1,106 @@
+const PREF_MULTISELECT_TABS = "browser.tabs.multiselect";
+const PREF_WARN_ON_CLOSE = "browser.tabs.warnOnCloseOtherTabs";
+
+add_task(async function setPref() {
+    await SpecialPowers.pushPrefEnv({
+        set: [
+            [PREF_MULTISELECT_TABS, true],
+            [PREF_WARN_ON_CLOSE, false]
+        ]
+    });
+});
+
+add_task(async function usingTabCloseButton() {
+    let tab1 = await addTab();
+    let tab2 = await addTab();
+    let tab3 = await addTab();
+    let tab4 = await addTab();
+
+    is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
+
+    await triggerClickOn(tab1, { ctrlKey: true });
+    await triggerClickOn(tab2, { ctrlKey: true });
+
+    ok(tab1.multiselected, "Tab1 is multiselected");
+    ok(tab2.multiselected, "Tab2 is multiselected");
+    ok(!tab3.multiselected, "Tab3 is not multiselected");
+    ok(!tab4.multiselected, "Tab4 is not multiselected");
+    is(gBrowser.multiSelectedTabsCount, 2, "Two multiselected tabs");
+
+    // Closing a tab which is not multiselected
+    let tab4CloseBtn = document.getAnonymousElementByAttribute(tab4, "anonid", "close-button");
+    let tab4Closing = BrowserTestUtils.waitForTabClosing(tab4);
+    tab4CloseBtn.click();
+    await tab4Closing;
+
+    ok(tab1.multiselected, "Tab1 is multiselected");
+    ok(!tab1.closing, "Tab1 is not closing");
+    ok(tab2.multiselected, "Tab2 is multiselected");
+    ok(!tab2.closing, "Tab2 is not closing");
+    ok(!tab3.multiselected, "Tab3 is not multiselected");
+    ok(!tab3.closing, "Tab3 is not closing");
+    ok(tab4.closing, "Tab4 is closing");
+    is(gBrowser.multiSelectedTabsCount, 2, "Two multiselected tabs");
+
+    // Closing a selected tab
+    let tab2CloseBtn = document.getAnonymousElementByAttribute(tab1, "anonid", "close-button");
+    let tab1Closing = BrowserTestUtils.waitForTabClosing(tab1);
+    let tab2Closing = BrowserTestUtils.waitForTabClosing(tab2);
+    tab2CloseBtn.click();
+    await tab1Closing;
+    await tab2Closing;
+
+    ok(tab1.closing, "Tab1 is closing");
+    ok(tab2.closing, "Tab2 is closing");
+    ok(!tab3.closing, "Tab3 is not closing");
+    is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
+
+    BrowserTestUtils.removeTab(tab3);
+});
+
+add_task(async function usingTabContextMenu() {
+    let tab1 = await addTab();
+    let tab2 = await addTab();
+    let tab3 = await addTab();
+    let tab4 = await addTab();
+
+    let menuItemCloseTab = document.getElementById("context_closeTab");
+    let menuItemCloseSelectedTabs = document.getElementById("context_closeSelectedTabs");
+
+    is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
+
+    // Check the context menu with zero multiselected tabs
+    updateTabContextMenu(tab4);
+    is(menuItemCloseTab.hidden, false, "Close Tab is visible");
+    is(menuItemCloseSelectedTabs.hidden, true, "Close Selected Tabs is hidden");
+
+    await triggerClickOn(tab1, { ctrlKey: true });
+    await triggerClickOn(tab2, { ctrlKey: true });
+
+    ok(tab1.multiselected, "Tab1 is multiselected");
+    ok(tab2.multiselected, "Tab2 is multiselected");
+    ok(!tab3.multiselected, "Tab3 is not multiselected");
+    ok(!tab4.multiselected, "Tab4 is not multiselected");
+    is(gBrowser.multiSelectedTabsCount, 2, "Two multiselected tabs");
+
+    // Check the context menu with two multiselected tabs
+    updateTabContextMenu(tab4);
+    is(menuItemCloseTab.hidden, true, "Close Tab is hidden");
+    is(menuItemCloseSelectedTabs.hidden, false, "Close Selected Tabs is visible");
+
+    let tab1Closing = BrowserTestUtils.waitForTabClosing(tab1);
+    let tab2Closing = BrowserTestUtils.waitForTabClosing(tab2);
+    menuItemCloseSelectedTabs.click();
+    await tab1Closing;
+    await tab2Closing;
+
+    ok(tab1.closing, "Tab1 is closing");
+    ok(tab2.closing, "Tab2 is closing");
+    ok(!tab3.closing, "Tab3 is not closing");
+    ok(!tab4.closing, "Tab4 is not closing");
+    is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
+
+    BrowserTestUtils.removeTab(tab3);
+    BrowserTestUtils.removeTab(tab4);
+});
+
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -29,16 +29,18 @@ a tab (i.e. it is a verb, not adjective)
 <!ENTITY  duplicateTab.accesskey             "D">
 <!-- LOCALIZATION NOTE (closeTabsToTheEnd.label): This should indicate the
 direction in which tabs are closed, i.e. locales that use RTL mode should say
 left instead of right. -->
 <!ENTITY  closeTabsToTheEnd.label            "Close Tabs to the Right">
 <!ENTITY  closeTabsToTheEnd.accesskey        "i">
 <!ENTITY  closeOtherTabs.label               "Close Other Tabs">
 <!ENTITY  closeOtherTabs.accesskey           "o">
+<!ENTITY  closeSelectedTabs.label            "Close Selected Tabs">
+<!ENTITY  closeSelectedTabs.accesskey        "S">
 
 <!-- LOCALIZATION NOTE (pinTab.label, unpinTab.label): "Pin" is being
 used as a metaphor for expressing the fact that these tabs are "pinned" to the
 left edge of the tabstrip. Really we just want the string to express the idea
 that this is a lightweight and reversible action that keeps your tab where you
 can reach it easily. -->
 <!ENTITY  pinTab.label                       "Pin Tab">
 <!ENTITY  pinTab.accesskey                   "P">