diff options
6 files changed, 68 insertions, 56 deletions
diff --git a/application/palemoon/components/places/content/browserPlacesViews.js b/application/palemoon/components/places/content/browserPlacesViews.js index eec7274a4..8b90dd280 100644 --- a/application/palemoon/components/places/content/browserPlacesViews.js +++ b/application/palemoon/components/places/content/browserPlacesViews.js @@ -109,8 +109,15 @@ PlacesViewBase.prototype = { get selectedNode() { if (this._contextMenuShown) { - let popup = document.popupNode; - return popup._placesNode || popup.parentNode._placesNode || null; + let anchor = this._contextMenuShown.triggerNode; + if (!anchor) + return null; + + if (anchor._placesNode) + return this._rootElt == anchor ? null : anchor._placesNode; + + anchor = anchor.parentNode; + return this._rootElt == anchor ? null : (anchor._placesNode || null); } return null; }, @@ -176,13 +183,13 @@ PlacesViewBase.prototype = { }, buildContextMenu: function PVB_buildContextMenu(aPopup) { - this._contextMenuShown = true; + this._contextMenuShown = aPopup; window.updateCommands("places"); return this.controller.buildContextMenu(aPopup); }, destroyContextMenu: function PVB_destroyContextMenu(aPopup) { - this._contextMenuShown = false; + this._contextMenuShown = null; }, _cleanPopup: function PVB_cleanPopup(aPopup, aDelay) { diff --git a/application/palemoon/components/places/content/controller.js b/application/palemoon/components/places/content/controller.js index 7f5f7f652..d09827163 100644 --- a/application/palemoon/components/places/content/controller.js +++ b/application/palemoon/components/places/content/controller.js @@ -334,20 +334,6 @@ PlacesController.prototype = { }, /** - * Determines whether or not the root node for the view is selected - */ - rootNodeIsSelected: function PC_rootNodeIsSelected() { - var nodes = this._view.selectedNodes; - var root = this._view.result.root; - for (var i = 0; i < nodes.length; ++i) { - if (nodes[i] == root) - return true; - } - - return false; - }, - - /** * Looks at the data on the clipboard to see if it is paste-able. * Paste-able data is: * - in a format that the view can receive @@ -400,7 +386,7 @@ PlacesController.prototype = { * Gathers information about the selected nodes according to the following * rules: * "link" node is a URI - * "bookmark" node is a bookamrk + * "bookmark" node is a bookmark * "livemarkChild" node is a child of a livemark * "tagChild" node is a child of a tag * "folder" node is a folder @@ -414,15 +400,10 @@ PlacesController.prototype = { * node are set on its corresponding object as properties. * Notes: * 1) This can be slow, so don't call it anywhere performance critical! - * 2) A single-object array corresponding the root node is returned if - * there's no selection. */ _buildSelectionMetadata: function PC__buildSelectionMetadata() { var metadata = []; - var root = this._view.result.root; var nodes = this._view.selectedNodes; - if (nodes.length == 0) - nodes.push(root); // See the second note above for (var i = 0; i < nodes.length; i++) { var nodeData = {}; @@ -501,10 +482,23 @@ PlacesController.prototype = { */ _shouldShowMenuItem: function PC__shouldShowMenuItem(aMenuItem, aMetaData) { var selectiontype = aMenuItem.getAttribute("selectiontype"); - if (selectiontype == "multiple" && aMetaData.length == 1) + if (!selectiontype) { + selectiontype = "single|multiple"; + } + var selectionTypes = selectiontype.split("|"); + if (selectionTypes.indexOf("any") != -1) { + return true; + } + var count = aMetaData.length; + if (count > 1 && selectionTypes.indexOf("multiple") == -1) return false; - if (selectiontype == "single" && aMetaData.length != 1) + if (count == 1 && selectionTypes.indexOf("single") == -1) return false; + // NB: if there is no selection, we show the item if (and only if) + // the selectiontype includes 'none' - the metadata list will be + // empty so none of the other criteria will apply anyway. + if (count == 0) + return selectionTypes.indexOf("none") != -1; var forceHideAttr = aMenuItem.getAttribute("forcehideselection"); if (forceHideAttr) { @@ -551,9 +545,11 @@ PlacesController.prototype = { * 1) The "selectiontype" attribute may be set on a menu-item to "single" * if the menu-item should be visible only if there is a single node * selected, or to "multiple" if the menu-item should be visible only if - * multiple nodes are selected. If the attribute is not set or if it is - * set to an invalid value, the menu-item may be visible for both types of - * selection. + * multiple nodes are selected, or to "none" if the menuitems should be + * visible for if there are no selected nodes, or to a |-separated + * combination of these. + * If the attribute is not set or set to an invalid value, the menu-item + * may be visible irrespective of the selection. * 2) The "selection" attribute may be set on a menu-item to the various * meta-data rules for which it may be visible. The rules should be * separated with the | character. @@ -584,7 +580,7 @@ PlacesController.prototype = { var separator = null; var visibleItemsBeforeSep = false; - var anyVisible = false; + var usableItemCount = 0; for (var i = 0; i < aPopup.childNodes.length; ++i) { var item = aPopup.childNodes[i]; if (item.localName != "menuseparator") { @@ -598,12 +594,13 @@ PlacesController.prototype = { (!/tree/i.test(this._view.localName) || ip); var hideIfPrivate = item.getAttribute("hideifprivatebrowsing") == "true" && PrivateBrowsingUtils.isWindowPrivate(window); - item.hidden = hideIfNoIP || hideIfPrivate || hideParentFolderItem || - !this._shouldShowMenuItem(item, metadata); + var shouldHideItem = hideIfNoIP || hideIfPrivate || hideParentFolderItem || + !this._shouldShowMenuItem(item, metadata); + item.hidden = item.disabled = shouldHideItem; if (!item.hidden) { visibleItemsBeforeSep = true; - anyVisible = true; + usableItemCount++; // Show the separator above the menu-item if any if (separator) { @@ -627,21 +624,21 @@ PlacesController.prototype = { } // Set Open Folder/Links In Tabs items enabled state if they're visible - if (anyVisible) { + if (usableItemCount > 0) { var openContainerInTabsItem = document.getElementById("placesContext_openContainer:tabs"); - if (!openContainerInTabsItem.hidden && this._view.selectedNode && - PlacesUtils.nodeIsContainer(this._view.selectedNode)) { - openContainerInTabsItem.disabled = - !PlacesUtils.hasChildURIs(this._view.selectedNode); - } - else { - // see selectiontype rule in the overlay - var openLinksInTabsItem = document.getElementById("placesContext_openLinks:tabs"); - openLinksInTabsItem.disabled = openLinksInTabsItem.hidden; + if (!openContainerInTabsItem.hidden) { + var containerToUse = this._view.selectedNode || this._view.result.root; + if (PlacesUtils.nodeIsContainer(containerToUse)) { + if (!PlacesUtils.hasChildURIs(containerToUse, true)) { + openContainerInTabsItem.disabled = true; + // Ensure that we don't display the menu if nothing is enabled: + usableItemCount--; + } + } } } - return anyVisible; + return usableItemCount > 0; }, /** @@ -707,10 +704,15 @@ PlacesController.prototype = { */ openSelectionInTabs: function PC_openLinksInTabs(aEvent) { var node = this._view.selectedNode; + var nodes = this._view.selectedNodes; + // In the case of no selection, open the root node: + if (!node && !nodes.length) { + node = this._view.result.root; + } if (node && PlacesUtils.nodeIsContainer(node)) - PlacesUIUtils.openContainerNodeInTabs(this._view.selectedNode, aEvent, this._view); + PlacesUIUtils.openContainerNodeInTabs(node, aEvent, this._view); else - PlacesUIUtils.openURINodesInTabs(this._view.selectedNodes, aEvent, this._view); + PlacesUIUtils.openURINodesInTabs(nodes, aEvent, this._view); }, /** diff --git a/application/palemoon/components/places/content/placesOverlay.xul b/application/palemoon/components/places/content/placesOverlay.xul index dd4d50f01..745990a9c 100644 --- a/application/palemoon/components/places/content/placesOverlay.xul +++ b/application/palemoon/components/places/content/placesOverlay.xul @@ -149,20 +149,20 @@ command="placesCmd_new:bookmark" label="&cmd.new_bookmark.label;" accesskey="&cmd.new_bookmark.accesskey;" - selection="any" + selectiontype="any" hideifnoinsertionpoint="true"/> <menuitem id="placesContext_new:folder" command="placesCmd_new:folder" label="&cmd.new_folder.label;" accesskey="&cmd.context_new_folder.accesskey;" - selection="any" + selectiontype="any" hideifnoinsertionpoint="true"/> <menuitem id="placesContext_new:separator" command="placesCmd_new:separator" label="&cmd.new_separator.label;" accesskey="&cmd.new_separator.accesskey;" closemenu="single" - selection="any" + selectiontype="any" hideifnoinsertionpoint="true"/> <menuseparator id="placesContext_newSeparator"/> <menuitem id="placesContext_createBookmark" @@ -182,14 +182,13 @@ command="placesCmd_copy" label="©Cmd.label;" closemenu="single" - accesskey="©Cmd.accesskey;" - selection="any"/> + accesskey="©Cmd.accesskey;"/> <menuitem id="placesContext_paste" command="placesCmd_paste" label="&pasteCmd.label;" closemenu="single" accesskey="&pasteCmd.accesskey;" - selection="any" + selectiontype="any" hideifnoinsertionpoint="true"/> <menuseparator id="placesContext_editSeparator"/> <menuitem id="placesContext_delete" diff --git a/application/palemoon/components/places/content/sidebarUtils.js b/application/palemoon/components/places/content/sidebarUtils.js index 8ffb70348..06ed53753 100644 --- a/application/palemoon/components/places/content/sidebarUtils.js +++ b/application/palemoon/components/places/content/sidebarUtils.js @@ -40,7 +40,7 @@ var SidebarUtils = { var openInTabs = isContainer && (aEvent.button == 1 || (aEvent.button == 0 && modifKey)) && - PlacesUtils.hasChildURIs(tbo.view.nodeForTreeIndex(cell.row)); + PlacesUtils.hasChildURIs(tbo.view.nodeForTreeIndex(cell.row), true); if (aEvent.button == 0 && isContainer && !openInTabs) { tbo.view.toggleOpenState(cell.row); diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index 323fa41a1..5f6e81f18 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1341,7 +1341,7 @@ this.PlacesUtils = { * The container node to search through. * @returns true if the node contains uri nodes, false otherwise. */ - hasChildURIs: function PU_hasChildURIs(aNode) { + hasChildURIs: function PU_hasChildURIs(aNode, aMultiple=false) { if (!this.nodeIsContainer(aNode)) return false; @@ -1357,11 +1357,14 @@ this.PlacesUtils = { root.containerOpen = true; } + let foundFirst = !aMultiple; let found = false; for (let i = 0; i < root.childCount && !found; i++) { let child = root.getChild(i); - if (this.nodeIsURI(child)) - found = true; + if (this.nodeIsURI(child)) { + found = foundFirst; + foundFirst = true; + } } if (!wasOpen) { diff --git a/toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js b/toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js index ecebce94a..3e2f88c21 100644 --- a/toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js +++ b/toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js @@ -165,6 +165,7 @@ function check_uri_nodes(aQuery, aOptions, aExpectedURINodes) { root.containerOpen = true; var node = root.getChild(0); do_check_eq(PU.hasChildURIs(node), aExpectedURINodes > 0); + do_check_eq(PU.hasChildURIs(node, true), aExpectedURINodes > 1); do_check_eq(PU.getURLsForContainerNode(node).length, aExpectedURINodes); root.containerOpen = false; } |