Bug 1458067 - Implement ability to bookmark a selection of tabs. r?Gijs,mak
MozReview-Commit-ID: 4KE3qSf3j0y
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -468,48 +468,64 @@ var PlacesCommandHook = {
uri: makeURI(url),
title,
defaultInsertionPoint,
hiddenRows: [ "location", "keyword" ]
}, window.top);
},
/**
- * List of nsIURI objects characterizing the tabs currently open in the
- * browser, modulo pinned tabs. The URIs will be in the order in which their
- * corresponding tabs appeared and duplicates are discarded.
- */
- get uniqueCurrentPages() {
+ * List of nsIURI objects characterizing tabs given in param.
+ * Duplicates are discarded.
+ */
+ getUniquePages(tabs) {
let uniquePages = {};
let URIs = [];
- gBrowser.visibleTabs.forEach(tab => {
+ tabs.forEach(tab => {
let browser = tab.linkedBrowser;
let uri = browser.currentURI;
let title = browser.contentTitle || tab.label;
let spec = uri.spec;
- if (!tab.pinned && !(spec in uniquePages)) {
+ if (!(spec in uniquePages)) {
uniquePages[spec] = null;
URIs.push({ uri, title });
}
});
return URIs;
},
/**
- * Adds a folder with bookmarks to all of the currently open tabs in this
- * window.
+ * List of nsIURI objects characterizing the tabs currently open in the
+ * browser, modulo pinned tabs. The URIs will be in the order in which their
+ * corresponding tabs appeared and duplicates are discarded.
+ */
+ get uniqueCurrentPages() {
+ let visibleUnpinnedTabs = gBrowser.visibleTabs
+ .filter(tab => !tab.pinned);
+ return this.getUniquePages(visibleUnpinnedTabs);
+ },
+
+ /**
+ * List of nsIURI objects characterizing the tabs currently
+ * selected in the window. Duplicates are discarded.
*/
- bookmarkCurrentPages: function PCH_bookmarkCurrentPages() {
- let pages = this.uniqueCurrentPages;
- if (pages.length > 1) {
- PlacesUIUtils.showBookmarkDialog({ action: "add",
- type: "folder",
- URIList: pages
- }, window);
+ get uniqueSelectedPages() {
+ return this.getUniquePages(gBrowser.selectedTabs);
+ },
+
+ /**
+ * Adds a folder with bookmarks to URIList given in param.
+ */
+ bookmarkPages(URIList) {
+ if (URIList.length > 1) {
+ PlacesUIUtils.showBookmarkDialog({ action: "add",
+ type: "folder",
+ URIList,
+ }, window);
}
},
/**
* Updates disabled state for the "Bookmark All Tabs" command.
*/
updateBookmarkAllTabsCommand:
function PCH_updateBookmarkAllTabsCommand() {
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -56,17 +56,17 @@
<command id="cmd_findSelection" oncommand="gLazyFindCommand('onFindSelectionCommand')"/>
#endif
<!-- work-around bug 392512 -->
<command id="Browser:AddBookmarkAs"
oncommand="PlacesCommandHook.bookmarkPage();"/>
<!-- The command disabled state must be manually updated through
PlacesCommandHook.updateBookmarkAllTabsCommand() -->
<command id="Browser:BookmarkAllTabs"
- oncommand="PlacesCommandHook.bookmarkCurrentPages();"/>
+ oncommand="PlacesCommandHook.bookmarkPages(PlacesCommandHook.uniqueCurrentPages);"/>
<command id="Browser:Home" oncommand="BrowserHome();"/>
<command id="Browser:Back" oncommand="BrowserBack();" disabled="true"/>
<command id="Browser:BackOrBackDuplicate" oncommand="BrowserBack(event);" disabled="true">
<observes element="Browser:Back" attribute="disabled"/>
</command>
<command id="Browser:Forward" oncommand="BrowserForward();" disabled="true"/>
<command id="Browser:ForwardOrForwardDuplicate" oncommand="BrowserForward(event);" disabled="true">
<observes element="Browser:Forward" attribute="disabled"/>
@@ -301,17 +301,17 @@
<key id="key_findSelection" key="&findSelectionCmd.commandkey;" command="cmd_findSelection" modifiers="accel"/>
#endif
<key keycode="&findAgainCmd.commandkey2;" command="cmd_findAgain"/>
<key keycode="&findAgainCmd.commandkey2;" command="cmd_findPrevious" modifiers="shift"/>
<key id="addBookmarkAsKb" key="&bookmarkThisPageCmd.commandkey;" command="Browser:AddBookmarkAs" modifiers="accel"/>
# Accel+Shift+A-F are reserved on GTK
#ifndef MOZ_WIDGET_GTK
- <key id="bookmarkAllTabsKb" key="&bookmarkThisPageCmd.commandkey;" oncommand="PlacesCommandHook.bookmarkCurrentPages();" modifiers="accel,shift"/>
+ <key id="bookmarkAllTabsKb" key="&bookmarkThisPageCmd.commandkey;" oncommand="PlacesCommandHook.bookmarkPages(PlacesCommandHook.uniqueCurrentPages);" modifiers="accel,shift"/>
<key id="manBookmarkKb" key="&bookmarksCmd.commandkey;" command="Browser:ShowAllBookmarks" modifiers="accel,shift"/>
#else
<key id="manBookmarkKb" key="&bookmarksGtkCmd.commandkey;" command="Browser:ShowAllBookmarks" modifiers="accel,shift"/>
#endif
<key id="viewBookmarksSidebarKb" key="&bookmarksCmd.commandkey;" command="viewBookmarksSidebar" modifiers="accel"/>
#ifdef XP_WIN
# Cmd+I is conventially mapped to Info on MacOS X, thus it should not be
# overridden for other purposes there.
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -127,16 +127,21 @@
accesskey="&sendTabToDevice.accesskey;">
<menupopup id="context_sendTabToDevicePopupMenu"
onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, TabContextMenu.contextTab.linkedBrowser.currentURI.spec, TabContextMenu.contextTab.linkedBrowser.contentTitle);"/>
</menu>
<menuseparator/>
<menuitem id="context_reloadAllTabs" label="&reloadAllTabs.label;" accesskey="&reloadAllTabs.accesskey;"
tbattr="tabbrowser-multiple-visible"
oncommand="gBrowser.reloadAllTabs();"/>
+ <menuitem id="context_bookmarkSelectedTabs"
+ hidden="true"
+ label="&bookmarkSelectedTabs.label;"
+ accesskey="&bookmarkSelectedTabs.accesskey;"
+ oncommand="PlacesCommandHook.bookmarkPages(PlacesCommandHook.uniqueSelectedPages);"/>
<menuitem id="context_bookmarkAllTabs"
label="&bookmarkAllTabs.label;"
accesskey="&bookmarkAllTabs.accesskey;"
command="Browser:BookmarkAllTabs"/>
<menuitem id="context_closeTabsToTheEnd" label="&closeTabsToTheEnd.label;" accesskey="&closeTabsToTheEnd.accesskey;"
oncommand="gBrowser.removeTabsToTheEndFrom(TabContextMenu.contextTab, {animate: true});"/>
<menuitem id="context_closeOtherTabs" label="&closeOtherTabs.label;" accesskey="&closeOtherTabs.accesskey;"
oncommand="gBrowser.removeAllTabsBut(TabContextMenu.contextTab);"/>
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -5022,23 +5022,28 @@ var TabContextMenu = {
unpinnedTabsToClose--;
}
document.getElementById("context_closeOtherTabs").disabled = unpinnedTabsToClose < 1;
// Only one of close_tab/close_selected_tabs should be visible
document.getElementById("context_closeTab").hidden = multiselectionContext;
document.getElementById("context_closeSelectedTabs").hidden = !multiselectionContext;
- // Hide "Bookmark All Tabs" for a pinned tab. Update its state if visible.
+ // Hide "Bookmark All Tabs" for a pinned tab or multiselection.
+ // Update its state if visible.
let bookmarkAllTabs = document.getElementById("context_bookmarkAllTabs");
- bookmarkAllTabs.hidden = this.contextTab.pinned;
+ bookmarkAllTabs.hidden = this.contextTab.pinned || multiselectionContext;
if (!bookmarkAllTabs.hidden) {
PlacesCommandHook.updateBookmarkAllTabsCommand();
}
+ // Show "Bookmark Selected Tabs" in a multiselect context and hide it otherwise.
+ let bookmarkMultiSelectedTabs = document.getElementById("context_bookmarkSelectedTabs");
+ bookmarkMultiSelectedTabs.hidden = !multiselectionContext;
+
let toggleMute = document.getElementById("context_toggleMuteTab");
let toggleMultiSelectMute = document.getElementById("context_toggleMuteSelectedTabs");
// Only one of mute_unmute_tab/mute_unmute_selected_tabs should be visible
toggleMute.hidden = multiselectionContext;
toggleMultiSelectMute.hidden = !multiselectionContext;
// Adjust the state of the toggle mute menu item.
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -15,16 +15,17 @@ tags = audiochannel
[browser_bug580956.js]
[browser_close_tab_by_dblclick.js]
[browser_contextmenu_openlink_after_tabnavigated.js]
skip-if = (verify && debug && (os == 'linux'))
support-files =
test_bug1358314.html
[browser_isLocalAboutURI.js]
[browser_multiselect_tabs_active_tab_selected_by_default.js]
+[browser_multiselect_tabs_bookmark.js]
[browser_multiselect_tabs_close_other_tabs.js]
[browser_multiselect_tabs_close_using_shortcuts.js]
[browser_multiselect_tabs_close.js]
[browser_multiselect_tabs_mute_unmute.js]
[browser_multiselect_tabs_pin_unpin.js]
[browser_multiselect_tabs_positional_attrs.js]
[browser_multiselect_tabs_reload.js]
[browser_multiselect_tabs_using_Ctrl.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_multiselect_tabs_bookmark.js
@@ -0,0 +1,64 @@
+/* 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/. */
+
+const PREF_MULTISELECT_TABS = "browser.tabs.multiselect";
+
+async function addTab_example_com() {
+ const tab = BrowserTestUtils.addTab(gBrowser,
+ "http://example.com/", { skipAnimation: true });
+ const browser = gBrowser.getBrowserForTab(tab);
+ await BrowserTestUtils.browserLoaded(browser);
+ return tab;
+}
+
+add_task(async function setPref() {
+ await SpecialPowers.pushPrefEnv({
+ set: [[PREF_MULTISELECT_TABS, true]]
+ });
+});
+
+add_task(async function test() {
+ let tab1 = await addTab();
+ let tab2 = await addTab();
+ let tab3 = await addTab();
+
+ let menuItemBookmarkAllTabs = document.getElementById("context_bookmarkAllTabs");
+ let menuItemBookmarkSelectedTabs = document.getElementById("context_bookmarkSelectedTabs");
+
+ is(gBrowser.multiSelectedTabsCount, 0, "Zero multiselected tabs");
+
+ await BrowserTestUtils.switchTab(gBrowser, tab1);
+ await triggerClickOn(tab2, { ctrlKey: true });
+
+ ok(tab1.multiselected, "Tab1 is multiselected");
+ ok(tab2.multiselected, "Tab2 is multiselected");
+ ok(!tab3.multiselected, "Tab3 is not multiselected");
+
+ // Check the context menu with a non-multiselected tab
+ updateTabContextMenu(tab3);
+ is(menuItemBookmarkAllTabs.hidden, false, "Bookmark All Tabs is visible");
+ is(menuItemBookmarkSelectedTabs.hidden, true, "Bookmark Selected Tabs is hidden");
+
+ // Check the context menu with a multiselected tab and one unique page in the selection.
+ updateTabContextMenu(tab2);
+ is(menuItemBookmarkAllTabs.hidden, true, "Bookmark All Tabs is visible");
+ is(menuItemBookmarkSelectedTabs.hidden, false, "Bookmark Selected Tabs is hidden");
+ is(PlacesCommandHook.uniqueSelectedPages.length, 1, "No more than one unique selected page");
+
+ info("Add a different page to selection");
+ let tab4 = await addTab_example_com();
+ await triggerClickOn(tab4, { ctrlKey: true });
+
+ ok(tab4.multiselected, "Tab4 is multiselected");
+ is(gBrowser.multiSelectedTabsCount, 3, "Three multiselected tabs");
+
+ // Check the context menu with a multiselected tab and two unique pages in the selection.
+ updateTabContextMenu(tab2);
+ is(menuItemBookmarkAllTabs.hidden, true, "Bookmark All Tabs is visible");
+ is(menuItemBookmarkSelectedTabs.hidden, false, "Bookmark Selected Tabs is hidden");
+ is(PlacesCommandHook.uniqueSelectedPages.length, 2, "More than one unique selected page");
+
+ for (let tab of [tab1, tab2, tab3, tab4])
+ BrowserTestUtils.removeTab(tab);
+});
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -44,16 +44,18 @@ but will never be visible at the same ti
<!ENTITY unpinSelectedTabs.label "Unpin Tabs">
<!-- Unpin Tab and Unpin Selected Tabs have the same accesskey
but will never be visible at the same time. -->
<!ENTITY unpinSelectedTabs.accesskey "b">
<!-- Reload Tab and Reload Selected Tabs have the same accesskey
but will never be visible at the same time. -->
<!ENTITY reloadSelectedTabs.label "Reload Selected Tabs">
<!ENTITY reloadSelectedTabs.accesskey "R">
+<!ENTITY bookmarkSelectedTabs.label "Bookmark Tabs…">
+<!ENTITY bookmarkSelectedTabs.accesskey "k">
<!-- 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">