--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -35,63 +35,16 @@ let gFaviconLoadDataMap = new Map();
const ITEM_CHANGED_BATCH_NOTIFICATION_THRESHOLD = 10;
// copied from utilityOverlay.js
const TAB_DROP_TYPE = "application/x-moz-tabbrowser-tab";
const PREF_LOAD_BOOKMARKS_IN_BACKGROUND = "browser.tabs.loadBookmarksInBackground";
const PREF_LOAD_BOOKMARKS_IN_TABS = "browser.tabs.loadBookmarksInTabs";
-// This function isn't public both because it's synchronous and because it is
-// going to be removed in bug 1072833.
-function IsLivemark(aItemId) {
- // Since this check may be done on each dragover event, it's worth maintaining
- // a cache.
- let self = IsLivemark;
- if (!("ids" in self)) {
- const LIVEMARK_ANNO = PlacesUtils.LMANNO_FEEDURI;
-
- let idsVec = PlacesUtils.annotations.getItemsWithAnnotation(LIVEMARK_ANNO);
- self.ids = new Set(idsVec);
-
- let obs = Object.freeze({
- QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarksObserver]),
-
- onItemChanged(itemId, property, isAnnoProperty, newValue, lastModified,
- itemType, parentId, guid) {
- if (isAnnoProperty && property == LIVEMARK_ANNO) {
- self.ids.add(itemId);
- }
- },
-
- onItemRemoved(itemId) {
- // Since the bookmark is removed, we know we can remove any references
- // to it from the cache.
- self.ids.delete(itemId);
- },
-
- onItemAdded() {},
- onBeginUpdateBatch() {},
- onEndUpdateBatch() {},
- onItemVisited() {},
- onItemMoved() {},
- onPageAnnotationSet() { },
- onPageAnnotationRemoved() { },
- skipDescendantsOnItemRemoval: false,
- skipTags: true,
- });
-
- PlacesUtils.bookmarks.addObserver(obs);
- PlacesUtils.registerShutdownFunction(() => {
- PlacesUtils.bookmarks.removeObserver(obs);
- });
- }
- return self.ids.has(aItemId);
-}
-
let InternalFaviconLoader = {
/**
* This gets called for every inner window that is destroyed.
* In the parent process, we process the destruction ourselves. In the child process,
* we notify the parent which will then process it based on that message.
*/
observe(subject, topic, data) {
let innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
@@ -809,19 +762,21 @@ this.PlacesUIUtils = {
},
/**
* Check whether or not the given node represents a removable entry (either in
* history or in bookmarks).
*
* @param aNode
* a node, except the root node of a query.
+ * @param aView
+ * The view originating the request.
* @return true if the aNode represents a removable entry, false otherwise.
*/
- canUserRemove(aNode) {
+ canUserRemove(aNode, aView) {
let parentNode = aNode.parent;
if (!parentNode) {
// canUserRemove doesn't accept root nodes.
return false;
}
// If it's not a bookmark, we can remove it unless it's a child of a
// livemark.
@@ -832,70 +787,66 @@ this.PlacesUIUtils = {
return !PlacesUtils.nodeIsFolder(parentNode);
}
// Generally it's always possible to remove children of a query.
if (PlacesUtils.nodeIsQuery(parentNode))
return true;
// Otherwise it has to be a child of an editable folder.
- return !this.isContentsReadOnly(parentNode);
+ return !this.isFolderReadOnly(parentNode, aView);
},
/**
* DO NOT USE THIS API IN ADDONS. IT IS VERY LIKELY TO CHANGE WHEN THE SWITCH
* TO GUIDS IS COMPLETE (BUG 1071511).
*
- * Check whether or not the given node or item-id points to a folder which
+ * Check whether or not the given Places node points to a folder which
* should not be modified by the user (i.e. its children should be unremovable
* and unmovable, new children should be disallowed, etc).
* These semantics are not inherited, meaning that read-only folder may
* contain editable items (for instance, the places root is read-only, but all
* of its direct children aren't).
*
- * You should only pass folder item ids or folder nodes for aNodeOrItemId.
- * While this is only enforced for the node case (if an item id of a separator
- * or a bookmark is passed, false is returned), it's considered the caller's
- * job to ensure that it checks a folder.
- * Also note that folder-shortcuts should only be passed as result nodes.
- * Otherwise they are just treated as bookmarks (i.e. false is returned).
+ * You should only pass folder nodes.
*
- * @param aNodeOrItemId
- * any item id or result node.
- * @throws if aNodeOrItemId is neither an item id nor a folder result node.
+ * @param placesNode
+ * any folder result node.
+ * @param view
+ * The view originating the request.
+ * @throws if placesNode is not a folder result node or views is invalid.
* @note livemark "folders" are considered read-only (but see bug 1072833).
- * @return true if aItemId points to a read-only folder, false otherwise.
+ * @return true if placesNode is a read-only folder, false otherwise.
*/
- isContentsReadOnly(aNodeOrItemId) {
- let itemId;
- if (typeof(aNodeOrItemId) == "number") {
- itemId = aNodeOrItemId;
- } else if (PlacesUtils.nodeIsFolder(aNodeOrItemId)) {
- itemId = PlacesUtils.getConcreteItemId(aNodeOrItemId);
- } else {
- throw new Error("invalid value for aNodeOrItemId");
+ isFolderReadOnly(placesNode, view) {
+ if (typeof placesNode != "object" || !PlacesUtils.nodeIsFolder(placesNode)) {
+ throw new Error("invalid value for placesNode");
}
-
- if (itemId == PlacesUtils.placesRootId || IsLivemark(itemId))
+ if (!view || typeof view != "object") {
+ throw new Error("invalid value for aView");
+ }
+ let itemId = PlacesUtils.getConcreteItemId(placesNode);
+ if (itemId == PlacesUtils.placesRootId ||
+ view.controller.hasCachedLivemarkInfo(placesNode))
return true;
// leftPaneFolderId, and as a result, allBookmarksFolderId, is a lazy getter
// performing at least a synchronous DB query (and on its very first call
// in a fresh profile, it also creates the entire structure).
// Therefore we don't want to this function, which is called very often by
// isCommandEnabled, to ever be the one that invokes it first, especially
// because isCommandEnabled may be called way before the left pane folder is
// even created (for example, if the user only uses the bookmarks menu or
// toolbar for managing bookmarks). To do so, we avoid comparing to those
// special folder if the lazy getter is still in place. This is safe merely
// because the only way to access the left pane contents goes through
// "resolving" the leftPaneFolderId getter.
- if ("get" in Object.getOwnPropertyDescriptor(this, "leftPaneFolderId"))
+ if (typeof Object.getOwnPropertyDescriptor(this, "leftPaneFolderId").get == "function") {
return false;
-
+ }
return itemId == this.leftPaneFolderId ||
itemId == this.allBookmarksFolderId;
},
/**
* Gives the user a chance to cancel loading lots of tabs at once
*/
confirmOpenInTabs(numTabsToOpen, aWindow) {--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -212,17 +212,17 @@ PlacesViewBase.prototype = {
tagName = container.title;
// TODO (Bug 1160193): properly support dropping on a tag root.
if (!tagName)
return null;
}
}
}
- if (PlacesControllerDragHelper.disallowInsertion(container))
+ if (PlacesControllerDragHelper.disallowInsertion(container, this))
return null;
return new InsertionPoint({
parentId: PlacesUtils.getConcreteItemId(container),
parentGuid: PlacesUtils.getConcreteItemGuid(container),
index, orientation, tagName
});
},
@@ -1494,17 +1494,17 @@ PlacesToolbar.prototype = {
let dropPoint = { ip: null, beforeIndex: null, folderElt: null };
let elt = aEvent.target;
if (elt._placesNode && elt != this._rootElt &&
elt.localName != "menupopup") {
let eltRect = elt.getBoundingClientRect();
let eltIndex = Array.prototype.indexOf.call(this._rootElt.childNodes, elt);
if (PlacesUtils.nodeIsFolder(elt._placesNode) &&
- !PlacesUIUtils.isContentsReadOnly(elt._placesNode)) {
+ !PlacesUIUtils.isFolderReadOnly(elt._placesNode, this)) {
// This is a folder.
// If we are in the middle of it, drop inside it.
// Otherwise, drop before it, with regards to RTL mode.
let threshold = eltRect.width * 0.25;
if (this.isRTL ? (aEvent.clientX > eltRect.right - threshold)
: (aEvent.clientX < eltRect.left + threshold)) {
// Drop before this folder.
dropPoint.ip =--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -195,17 +195,17 @@ PlacesController.prototype = {
// Livemark containers
let selectedNode = this._view.selectedNode;
return selectedNode && this.hasCachedLivemarkInfo(selectedNode);
}
case "placesCmd_sortBy:name": {
let selectedNode = this._view.selectedNode;
return selectedNode &&
PlacesUtils.nodeIsFolder(selectedNode) &&
- !PlacesUIUtils.isContentsReadOnly(selectedNode) &&
+ !PlacesUIUtils.isFolderReadOnly(selectedNode, this._view) &&
this._view.result.sortingMode ==
Ci.nsINavHistoryQueryOptions.SORT_BY_NONE;
}
case "placesCmd_createBookmark":
var node = this._view.selectedNode;
return node && PlacesUtils.nodeIsURI(node) && node.itemId == -1;
default:
return false;
@@ -327,17 +327,17 @@ PlacesController.prototype = {
for (var j = 0; j < ranges.length; j++) {
var nodes = ranges[j];
for (var i = 0; i < nodes.length; ++i) {
// Disallow removing the view's root node
if (nodes[i] == root)
return false;
- if (!PlacesUIUtils.canUserRemove(nodes[i]))
+ if (!PlacesUIUtils.canUserRemove(nodes[i], this._view))
return false;
}
}
return true;
},
/**
@@ -1545,55 +1545,72 @@ var PlacesControllerDragHelper = {
}
}
return true;
},
/**
* Determines if an unwrapped node can be moved.
*
- * @param aUnwrappedNode
- * A node unwrapped by PlacesUtils.unwrapNodes().
+ * @param unwrappedNode
+ * A node unwrapped by PlacesUtils.unwrapNodes().
* @return True if the node can be moved, false otherwise.
*/
- canMoveUnwrappedNode(aUnwrappedNode) {
- return aUnwrappedNode.id > 0 &&
- !PlacesUtils.isRootItem(aUnwrappedNode.id) &&
- (!aUnwrappedNode.parent || !PlacesUIUtils.isContentsReadOnly(aUnwrappedNode.parent)) &&
- aUnwrappedNode.parent != PlacesUtils.tagsFolderId &&
- aUnwrappedNode.grandParentId != PlacesUtils.tagsFolderId;
+ canMoveUnwrappedNode(unwrappedNode) {
+ if (unwrappedNode.id <= 0 || PlacesUtils.isRootItem(unwrappedNode.id)) {
+ return false;
+ }
+ let parentId = unwrappedNode.parent;
+ if (parentId <= 0 ||
+ parentId == PlacesUtils.placesRootId ||
+ parentId == PlacesUtils.tagsFolderId ||
+ unwrappedNode.grandParentId == PlacesUtils.tagsFolderId) {
+ return false;
+ }
+ // leftPaneFolderId and allBookmarksFolderId are lazy getters running
+ // at least a synchronous DB query. Therefore we don't want to invoke
+ // them first, especially because isCommandEnabled may be called way
+ // before the left pane folder is even necessary.
+ if (typeof Object.getOwnPropertyDescriptor(PlacesUIUtils, "leftPaneFolderId").get != "function" &&
+ (parentId == PlacesUIUtils.leftPaneFolderId ||
+ parentId == PlacesUIUtils.allBookmarksFolderId)) {
+ return false;
+ }
+ return true;
},
/**
* Determines if a node can be moved.
*
* @param aNode
* A nsINavHistoryResultNode node.
+ * @param aView
+ * The view originating the request
* @param [optional] aDOMNode
* A XUL DOM node.
* @return True if the node can be moved, false otherwise.
*/
- canMoveNode(aNode, aDOMNode) {
+ canMoveNode(aNode, aView, aDOMNode) {
// Only bookmark items are movable.
if (aNode.itemId == -1)
return false;
let parentNode = aNode.parent;
if (!parentNode) {
// Normally parentless places nodes can not be moved,
// but simulated bookmarked URI nodes are special.
return !!aDOMNode &&
aDOMNode.hasAttribute("simulated-places-node") &&
PlacesUtils.nodeIsBookmark(aNode);
}
// Once tags and bookmarked are divorced, the tag-query check should be
// removed.
- return !(PlacesUtils.nodeIsFolder(parentNode) &&
- PlacesUIUtils.isContentsReadOnly(parentNode)) &&
+ return PlacesUtils.nodeIsFolder(parentNode) &&
+ !PlacesUIUtils.isFolderReadOnly(parentNode, aView) &&
!PlacesUtils.nodeIsTagQuery(parentNode);
},
/**
* Handles the drop of one or more items onto a view.
*
* @param {Object} insertionPoint The insertion point where the items should
* be dropped.
@@ -1733,23 +1750,25 @@ var PlacesControllerDragHelper = {
PlacesUtils.transactionManager.doTransaction(txn);
}
},
/**
* Checks if we can insert into a container.
* @param aContainer
* The container were we are want to drop
+ * @param aView
+ * The view generating the request
*/
- disallowInsertion(aContainer) {
+ disallowInsertion(aContainer, aView) {
NS_ASSERT(aContainer, "empty container");
// Allow dropping into Tag containers and editable folders.
return !PlacesUtils.nodeIsTagQuery(aContainer) &&
(!PlacesUtils.nodeIsFolder(aContainer) ||
- PlacesUIUtils.isContentsReadOnly(aContainer));
+ PlacesUIUtils.isFolderReadOnly(aContainer, aView));
}
};
XPCOMUtils.defineLazyServiceGetter(PlacesControllerDragHelper, "dragService",
"@mozilla.org/widget/dragservice;1",
"nsIDragService");
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -50,18 +50,24 @@ var gEditItemOverlay = {
let parentId = -1;
let parentGuid = null;
if (node && isItem) {
if (!node.parent || (node.parent.itemId > 0 && !node.parent.bookmarkGuid)) {
throw new Error("Cannot use an incomplete node to initialize the edit bookmark panel");
}
let parent = node.parent;
- isParentReadOnly = !PlacesUtils.nodeIsFolder(parent) ||
- PlacesUIUtils.isContentsReadOnly(parent);
+ isParentReadOnly = !PlacesUtils.nodeIsFolder(parent);
+ if (!isParentReadOnly) {
+ let folderId = PlacesUtils.getConcreteItemId(parent);
+ isParentReadOnly = folderId == PlacesUtils.placesRootId ||
+ (!("get" in Object.getOwnPropertyDescriptor(PlacesUIUtils, "leftPaneFolderId")) &&
+ (folderId == PlacesUIUtils.leftPaneFolderId ||
+ folderId == PlacesUIUtils.allBookmarksFolderId));
+ }
parentId = parent.itemId;
parentGuid = parent.bookmarkGuid;
}
let focusedElement = aInitInfo.focusedElement;
let onPanelReady = aInitInfo.onPanelReady;
return this._paneInfo = { itemId, itemGuid, parentId, parentGuid, isItem,--- a/browser/components/places/content/menu.xml
+++ b/browser/components/places/content/menu.xml
@@ -64,17 +64,17 @@
dragging over this popup insertion point -->
<method name="_getDropPoint">
<parameter name="aEvent"/>
<body><![CDATA[
// Can't drop if the menu isn't a folder
let resultNode = this._placesNode;
if (!PlacesUtils.nodeIsFolder(resultNode) ||
- PlacesControllerDragHelper.disallowInsertion(resultNode)) {
+ PlacesControllerDragHelper.disallowInsertion(resultNode, this._rootView)) {
return null;
}
var dropPoint = { ip: null, folderElt: null };
// The element we are dragging over
let elt = aEvent.target;
if (elt.localName == "menupopup")
@@ -103,17 +103,17 @@
elt.lastChild.hasAttribute("placespopup"))
dropPoint.folderElt = elt;
return dropPoint;
}
let tagName = PlacesUtils.nodeIsTagQuery(elt._placesNode) ?
elt._placesNode.title : null;
if ((PlacesUtils.nodeIsFolder(elt._placesNode) &&
- !PlacesUIUtils.isContentsReadOnly(elt._placesNode)) ||
+ !PlacesUIUtils.isFolderReadOnly(elt._placesNode, this._rootView)) ||
PlacesUtils.nodeIsTagQuery(elt._placesNode)) {
// This is a folder or a tag container.
if (eventY - eltY < eltHeight * 0.20) {
// If mouse is in the top part of the element, drop above folder.
dropPoint.ip = new InsertionPoint({
parentId: PlacesUtils.getConcreteItemId(resultNode),
parentGuid: PlacesUtils.getConcreteItemGuid(resultNode),
orientation: Ci.nsITreeView.DROP_BEFORE,
@@ -345,17 +345,17 @@
let elt = event.target;
if (!elt._placesNode)
return;
let draggedElt = elt._placesNode;
// Force a copy action if parent node is a query or we are dragging a
// not-removable node.
- if (!PlacesControllerDragHelper.canMoveNode(draggedElt, elt))
+ if (!PlacesControllerDragHelper.canMoveNode(draggedElt, this._rootView, elt))
event.dataTransfer.effectAllowed = "copyLink";
// Activate the view and cache the dragged element.
this._rootView._draggedElt = draggedElt;
this._rootView.controller.setDataTransfer(event);
this.setAttribute("dragstart", "true");
event.stopPropagation();
]]></handler>--- a/browser/components/places/content/tree.xml
+++ b/browser/components/places/content/tree.xml
@@ -495,17 +495,17 @@
container = lastSelected.parent;
// See comment in the treeView.js's copy of this method
if (!container || !container.containerOpen)
return null;
// Avoid the potentially expensive call to getChildIndex
// if we know this container doesn't allow insertion
- if (PlacesControllerDragHelper.disallowInsertion(container))
+ if (PlacesControllerDragHelper.disallowInsertion(container, this))
return null;
var queryOptions = PlacesUtils.asQuery(result.root).queryOptions;
if (queryOptions.sortingMode !=
Ci.nsINavHistoryQueryOptions.SORT_BY_NONE) {
// If we are within a sorted view, insert at the end
index = -1;
} else if (queryOptions.excludeItems ||
@@ -518,17 +518,17 @@
dropNearNode = lastSelected;
} else {
var lsi = container.getChildIndex(lastSelected);
index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
}
}
}
- if (PlacesControllerDragHelper.disallowInsertion(container))
+ if (PlacesControllerDragHelper.disallowInsertion(container, this))
return null;
// TODO (Bug 1160193): properly support dropping on a tag root.
let tagName = null;
if (PlacesUtils.nodeIsTagQuery(container)) {
tagName = container.title;
if (!tagName)
return null;
@@ -728,17 +728,17 @@
if (!node.parent) {
event.preventDefault();
event.stopPropagation();
return;
}
// If this node is child of a readonly container (e.g. a livemark)
// or cannot be moved, we must force a copy.
- if (!PlacesControllerDragHelper.canMoveNode(node)) {
+ if (!PlacesControllerDragHelper.canMoveNode(node, this)) {
event.dataTransfer.effectAllowed = "copyLink";
break;
}
}
this._controller.setDataTransfer(event);
event.stopPropagation();
]]></handler>--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1401,17 +1401,17 @@ PlacesTreeView.prototype = {
// container here. However, we can simply bail out when this happens,
// because we would then be back here in less than a millisecond, when
// the container had been reopened.
if (!container || !container.containerOpen)
return null;
// Avoid the potentially expensive call to getChildIndex
// if we know this container doesn't allow insertion.
- if (PlacesControllerDragHelper.disallowInsertion(container))
+ if (PlacesControllerDragHelper.disallowInsertion(container, this._tree.element))
return null;
let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions;
if (queryOptions.sortingMode !=
Ci.nsINavHistoryQueryOptions.SORT_BY_NONE) {
// If we are within a sorted view, insert at the end.
index = -1;
} else if (queryOptions.excludeItems ||
@@ -1424,17 +1424,17 @@ PlacesTreeView.prototype = {
dropNearNode = lastSelected;
} else {
let lsi = container.getChildIndex(lastSelected);
index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
}
}
}
- if (PlacesControllerDragHelper.disallowInsertion(container))
+ if (PlacesControllerDragHelper.disallowInsertion(container, this._tree.element))
return null;
// TODO (Bug 1160193): properly support dropping on a tag root.
let tagName = null;
if (PlacesUtils.nodeIsTagQuery(container)) {
tagName = container.title;
if (!tagName)
return null;--- a/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js
+++ b/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js
@@ -1,185 +1,125 @@
/* vim:set ts=2 sw=2 sts=2 et: */
/* 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/. */
"use strict";
-let rootFolder;
-let rootNode;
-
-add_task(async function setup() {
- rootFolder = await PlacesUtils.bookmarks.insert({
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+add_task(async function() {
+ let root = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "",
- type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ type: PlacesUtils.bookmarks.TYPE_FOLDER
});
- let rootId = await PlacesUtils.promiseItemId(rootFolder.guid);
- rootNode = PlacesUtils.getFolderContents(rootId, false, true).root;
-
- Assert.equal(rootNode.childCount, 0, "confirm test root is empty");
-
registerCleanupFunction(async () => {
await PlacesUtils.bookmarks.eraseEverything();
});
-});
-add_task(async function test_regular_folder() {
- let regularFolder = await PlacesUtils.bookmarks.insert({
- parentGuid: rootFolder.guid,
- title: "",
- type: PlacesUtils.bookmarks.TYPE_FOLDER,
- });
+ await withSidebarTree("bookmarks", async function(tree) {
+ info("Test a regular folder");
+ let folder = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "",
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ });
+ let folderId = await PlacesUtils.promiseItemId(folder.guid);
+ tree.selectItems([folderId]);
+ Assert.equal(tree.selectedNode.bookmarkGuid, folder.guid,
+ "Selected the expected node");
+ Assert.equal(tree.selectedNode.type, 6, "node is a folder");
+ Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree),
+ "can move regular folder node");
- Assert.equal(rootNode.childCount, 1,
- "populate added data to the test root");
- Assert.equal(PlacesControllerDragHelper.canMoveNode(rootNode.getChild(0)),
- true, "can move regular folder node");
-
- await PlacesUtils.bookmarks.remove(regularFolder);
-});
-
-add_task(async function test_folder_shortcut() {
- let regularFolder = await PlacesUtils.bookmarks.insert({
- parentGuid: rootFolder.guid,
- title: "",
- type: PlacesUtils.bookmarks.TYPE_FOLDER,
- });
- let regularFolderId = await PlacesUtils.promiseItemId(regularFolder.guid);
+ info("Test a folder shortcut");
+ let shortcut = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "bar",
+ url: `place:folder=${folderId}`
+ });
+ let shortcutId = await PlacesUtils.promiseItemId(shortcut.guid);
+ tree.selectItems([shortcutId]);
+ Assert.equal(tree.selectedNode.bookmarkGuid, shortcut.guid,
+ "Selected the expected node");
+ Assert.equal(tree.selectedNode.type, 9, "node is a folder shortcut");
+ Assert.equal(PlacesUtils.getConcreteItemGuid(tree.selectedNode),
+ folder.guid, "shortcut node guid and concrete guid match");
+ Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree),
+ "can move folder shortcut node");
- let shortcut = await PlacesUtils.bookmarks.insert({
- parentGuid: rootFolder.guid,
- title: "bar",
- url: `place:folder=${regularFolderId}`
- });
+ info("Test a query");
+ let bookmark = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "",
+ url: "http://foo.com",
+ });
+ let bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid);
+ tree.selectItems([bookmarkId]);
+ Assert.equal(tree.selectedNode.bookmarkGuid, bookmark.guid,
+ "Selected the expected node");
+ let query = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "bar",
+ url: `place:terms=foo`
+ });
+ let queryId = await PlacesUtils.promiseItemId(query.guid);
+ tree.selectItems([queryId]);
+ Assert.equal(tree.selectedNode.bookmarkGuid, query.guid,
+ "Selected the expected node");
+ Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree),
+ "can move query node");
- Assert.equal(rootNode.childCount, 2,
- "populated data to the test root");
-
- let folderNode = rootNode.getChild(0);
- Assert.equal(folderNode.type, 6, "node is folder");
- Assert.equal(regularFolder.guid, folderNode.bookmarkGuid, "folder guid and folder node item guid match");
- let shortcutNode = rootNode.getChild(1);
- Assert.equal(shortcutNode.type, 9, "node is folder shortcut");
- Assert.equal(shortcut.guid, shortcutNode.bookmarkGuid, "shortcut guid and shortcut node item guid match");
+ info("Test a tag container");
+ PlacesUtils.tagging.tagURI(Services.io.newURI(bookmark.url.href), ["bar"]);
+ // Add the tags root query.
+ let tagsQuery = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "",
+ url: "place:type=" + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY,
+ });
+ let tagsQueryId = await PlacesUtils.promiseItemId(tagsQuery.guid);
+ tree.selectItems([tagsQueryId]);
+ PlacesUtils.asQuery(tree.selectedNode).containerOpen = true;
+ Assert.equal(tree.selectedNode.childCount, 1, "has tags");
+ let tagNode = tree.selectedNode.getChild(0);
+ Assert.ok(!PlacesControllerDragHelper.canMoveNode(tagNode, tree),
+ "should not be able to move tag container node");
+ tree.selectedNode.containerOpen = false;
- let concreteId = PlacesUtils.getConcreteItemGuid(shortcutNode);
- Assert.equal(concreteId, folderNode.bookmarkGuid, "shortcut node id and concrete id match");
+ info("Test that special folders and cannot be moved but other shortcuts can.");
+ let roots = [
+ PlacesUtils.bookmarksMenuFolderId,
+ PlacesUtils.unfiledBookmarksFolderId,
+ PlacesUtils.toolbarFolderId,
+ ];
- Assert.equal(PlacesControllerDragHelper.canMoveNode(shortcutNode), true,
- "can move folder shortcut node");
-
- await PlacesUtils.bookmarks.remove(shortcut);
- await PlacesUtils.bookmarks.remove(regularFolder);
+ for (let id of roots) {
+ selectShortcutForRootId(tree, id);
+ Assert.ok(!PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree),
+ "shouldn't be able to move default shortcuts to roots");
+ let s = await PlacesUtils.bookmarks.insert({
+ parentGuid: root.guid,
+ title: "bar",
+ url: `place:folder=${id}`,
+ });
+ let sid = await PlacesUtils.promiseItemId(s.guid);
+ tree.selectItems([sid]);
+ Assert.equal(tree.selectedNode.bookmarkGuid, s.guid,
+ "Selected the expected node");
+ Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree),
+ "should be able to move user-created shortcuts to roots");
+ }
+ });
});
-add_task(async function test_regular_query() {
- let bookmark = await PlacesUtils.bookmarks.insert({
- parentGuid: rootFolder.guid,
- title: "",
- url: "http://foo.com",
- });
-
- let query = await PlacesUtils.bookmarks.insert({
- parentGuid: rootFolder.guid,
- title: "bar",
- url: `place:terms=foo`
- });
-
- Assert.equal(rootNode.childCount, 2,
- "populated data to the test root");
-
- let bmNode = rootNode.getChild(0);
- Assert.equal(bmNode.bookmarkGuid, bookmark.guid, "bookmark guid and bookmark node item guid match");
-
- let queryNode = rootNode.getChild(1);
- Assert.equal(queryNode.bookmarkGuid, query.guid, "query guid and query node item guid match");
-
- Assert.equal(PlacesControllerDragHelper.canMoveNode(queryNode),
- true, "can move query node");
-
- await PlacesUtils.bookmarks.remove(query);
- await PlacesUtils.bookmarks.remove(bookmark);
-});
-
-add_task(async function test_special_folders() {
- // Test that special folders and special folder shortcuts cannot be moved.
- let folders = [
- PlacesUtils.bookmarksMenuFolderId,
- PlacesUtils.tagsFolderId,
- PlacesUtils.unfiledBookmarksFolderId,
- PlacesUtils.toolbarFolderId
- ];
-
- let children = folders.map(folderId => {
- return {
- title: "",
- url: `place:folder=${folderId}`
- };
- });
-
- let shortcuts = await PlacesUtils.bookmarks.insertTree({
- guid: rootFolder.guid,
- children
- });
-
- // test toolbar shortcut node
- Assert.equal(rootNode.childCount, folders.length,
- "populated data to the test root");
-
- function getRootChildNode(aId) {
- let node = PlacesUtils.getFolderContents(PlacesUtils.placesRootId, false, true).root;
- for (let i = 0; i < node.childCount; i++) {
- let child = node.getChild(i);
- if (child.itemId == aId) {
- node.containerOpen = false;
- return child;
- }
+function selectShortcutForRootId(tree, id) {
+ for (let i = 0; i < tree.result.root.childCount; ++i) {
+ let child = tree.result.root.getChild(i);
+ if (PlacesUtils.getConcreteItemId(child) == id) {
+ tree.selectItems([child.itemId]);
+ return;
}
- node.containerOpen = false;
- ok(false, "Unable to find child node");
- return null;
}
-
- for (let i = 0; i < folders.length; i++) {
- let id = folders[i];
-
- let node = getRootChildNode(id);
- isnot(node, null, "Node found");
- Assert.equal(PlacesControllerDragHelper.canMoveNode(node),
- false, "shouldn't be able to move special folder node");
-
- let shortcut = shortcuts[i];
- let shortcutNode = rootNode.getChild(i);
-
- Assert.equal(shortcutNode.bookmarkGuid, shortcut.guid,
- "shortcut guid and shortcut node item guid match");
-
- Assert.equal(PlacesControllerDragHelper.canMoveNode(shortcutNode),
- true, "should be able to move special folder shortcut node");
- }
-});
-
-add_task(async function test_tag_container() {
- // tag a uri
- this.uri = makeURI("http://foo.com");
- PlacesUtils.tagging.tagURI(this.uri, ["bar"]);
- registerCleanupFunction(() => PlacesUtils.tagging.untagURI(this.uri, ["bar"]));
-
- // get tag root
- let query = PlacesUtils.history.getNewQuery();
- let options = PlacesUtils.history.getNewQueryOptions();
- options.resultType = Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY;
- let tagsNode = PlacesUtils.history.executeQuery(query, options).root;
-
- tagsNode.containerOpen = true;
- Assert.equal(tagsNode.childCount, 1, "has new tag");
-
- let tagNode = tagsNode.getChild(0);
-
- Assert.equal(PlacesControllerDragHelper.canMoveNode(tagNode),
- false, "should not be able to move tag container node");
- tagsNode.containerOpen = false;
-});
+ Assert.ok(false, "Cannot find shortcut to root");
+}--- a/browser/components/places/tests/browser/browser_library_left_pane_fixnames.js
+++ b/browser/components/places/tests/browser/browser_library_left_pane_fixnames.js
@@ -76,13 +76,13 @@ function test() {
}
leftPaneQueries.push(query);
// Rename to a bad title.
PlacesUtils.bookmarks.setItemTitle(query.itemId, "badName");
if ("concreteId" in query)
PlacesUtils.bookmarks.setItemTitle(query.concreteId, "badName");
}
- PlacesUIUtils.__defineGetter__("leftPaneFolderId", cachedLeftPaneFolderIdGetter);
+ restoreLeftPaneGetters();
// Open Library, this will kick-off left pane code.
openLibrary(onLibraryReady);
}--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -1,29 +1,36 @@
XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
"resource://gre/modules/NetUtil.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils",
"resource://testing-common/PlacesTestUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TestUtils",
"resource://testing-common/TestUtils.jsm");
-// We need to cache this before test runs...
-var cachedLeftPaneFolderIdGetter;
-var getter = PlacesUIUtils.__lookupGetter__("leftPaneFolderId");
-if (!cachedLeftPaneFolderIdGetter && typeof(getter) == "function") {
- cachedLeftPaneFolderIdGetter = getter;
+// We need to cache these before test runs...
+let leftPaneGetters = new Map([["leftPaneFolderId", null],
+ ["allBookmarksFolderId", null]]);
+for (let [key, val] of leftPaneGetters) {
+ if (!val) {
+ let getter = Object.getOwnPropertyDescriptor(PlacesUIUtils, key).get;
+ if (typeof getter == "function") {
+ leftPaneGetters.set(key, getter);
+ }
+ }
}
-// ...And restore it when test ends.
-registerCleanupFunction(function() {
- let updatedGetter = PlacesUIUtils.__lookupGetter__("leftPaneFolderId");
- if (cachedLeftPaneFolderIdGetter && typeof(updatedGetter) != "function") {
- PlacesUIUtils.__defineGetter__("leftPaneFolderId", cachedLeftPaneFolderIdGetter);
+// ...And restore them when test ends.
+function restoreLeftPaneGetters() {
+ for (let [key, getter] of leftPaneGetters) {
+ Object.defineProperty(PlacesUIUtils, key, {
+ enumerable: true, configurable: true, get: getter
+ });
}
-});
+}
+registerCleanupFunction(restoreLeftPaneGetters);
function openLibrary(callback, aLeftPaneRoot) {
let library = window.openDialog("chrome://browser/content/places/places.xul",
"", "chrome,toolbar=yes,dialog=no,resizable",
aLeftPaneRoot);
waitForFocus(function() {
callback(library);
}, library);--- a/browser/components/places/tests/chrome/test_0_bug510634.xul
+++ b/browser/components/places/tests/chrome/test_0_bug510634.xul
@@ -39,29 +39,39 @@
*
* Ensures that properties for special queries are set on their tree nodes,
* even if PlacesUIUtils.leftPaneFolderId was not initialized.
*/
SimpleTest.waitForExplicitFinish();
function runTest() {
- // We need to cache and restore this getter in order to simulate
- // Bug 510634
- let cachedLeftPaneFolderIdGetter =
- PlacesUIUtils.__lookupGetter__("leftPaneFolderId");
- // Must also cache and restore this getter as it is affected by
- // leftPaneFolderId, from bug 564900.
- let cachedAllBookmarksFolderIdGetter =
- PlacesUIUtils.__lookupGetter__("allBookmarksFolderId");
+ // We need to cache and restore the getters in order to simulate
+ // Bug 510634.
+ let leftPaneGetters = new Map([["leftPaneFolderId", null],
+ ["allBookmarksFolderId", null]]);
+ for (let [key, val] of leftPaneGetters) {
+ if (!val) {
+ let getter = Object.getOwnPropertyDescriptor(PlacesUIUtils, key).get;
+ if (typeof getter == "function") {
+ leftPaneGetters.set(key, getter);
+ }
+ }
+ }
+
+ function restoreLeftPaneGetters() {
+ for (let [key, getter] of leftPaneGetters) {
+ Object.defineProperty(PlacesUIUtils, key, {
+ enumerable: true, configurable: true, get: getter
+ });
+ }
+ }
let leftPaneFolderId = PlacesUIUtils.leftPaneFolderId;
-
- // restore the getter
- PlacesUIUtils.__defineGetter__("leftPaneFolderId", cachedLeftPaneFolderIdGetter);
+ restoreLeftPaneGetters();
// Setup the places tree contents.
let tree = document.getElementById("tree");
tree.place = "place:queryType=1&folder=" + leftPaneFolderId;
// The query-property is set on the title column for each row.
let titleColumn = tree.treeBoxObject.columns.getColumnAt(0);
@@ -82,18 +92,16 @@
ok(found, "OrganizerQuery_" + aQueryName + " is set");
}
);
// Close the root node
tree.result.root.containerOpen = false;
// Restore the getters for the next test.
- PlacesUIUtils.__defineGetter__("leftPaneFolderId", cachedLeftPaneFolderIdGetter);
- PlacesUIUtils.__defineGetter__("allBookmarksFolderId",
- cachedAllBookmarksFolderIdGetter);
+ restoreLeftPaneGetters();
SimpleTest.finish();
}
]]>
</script>
</window>deleted file mode 100644
--- a/browser/components/places/tests/unit/test_PUIU_livemarksCache.js
+++ /dev/null
@@ -1,40 +0,0 @@
-"use strict";
-
-let {IsLivemark} = Cu.import("resource:///modules/PlacesUIUtils.jsm", {});
-
-add_task(function test_livemark_cache_builtin_folder() {
- // This test checks a basic livemark, and also initializes the observer for
- // updates to the bookmarks.
- Assert.ok(!IsLivemark(PlacesUtils.unfiledBookmarksFolderId),
- "unfiled bookmarks should not be seen as a livemark");
-});
-
-add_task(async function test_livemark_add_and_remove_items() {
- let bookmark = await PlacesUtils.bookmarks.insert({
- parentGuid: PlacesUtils.bookmarks.unfiledGuid,
- title: "Grandsire",
- url: "http://example.com",
- });
-
- let bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid);
-
- Assert.ok(!IsLivemark(bookmarkId),
- "a simple bookmark should not be seen as a livemark");
-
- let livemark = await PlacesUtils.livemarks.addLivemark({
- title: "Stedman",
- feedURI: Services.io.newURI("http://livemark.com/"),
- parentGuid: PlacesUtils.bookmarks.unfiledGuid,
- });
-
- let livemarkId = await PlacesUtils.promiseItemId(livemark.guid);
-
- Assert.ok(IsLivemark(livemarkId),
- "a livemark should be reported as a livemark");
-
- // Now remove the livemark.
- await PlacesUtils.livemarks.removeLivemark(livemark);
-
- Assert.ok(!IsLivemark(livemarkId),
- "the livemark should have been removed from the cache");
-});--- a/browser/components/places/tests/unit/xpcshell.ini
+++ b/browser/components/places/tests/unit/xpcshell.ini
@@ -17,10 +17,9 @@ support-files =
[test_browserGlue_migrate.js]
[test_browserGlue_prefs.js]
[test_browserGlue_restore.js]
[test_browserGlue_smartBookmarks.js]
[test_browserGlue_urlbar_defaultbehavior_migration.js]
[test_clearHistory_shutdown.js]
[test_leftpane_corruption_handling.js]
[test_PUIU_batchUpdatesForNode.js]
-[test_PUIU_livemarksCache.js]
[test_PUIU_makeTransaction.js]
--- a/toolkit/components/places/nsLivemarkService.js
+++ b/toolkit/components/places/nsLivemarkService.js
@@ -349,16 +349,19 @@ LivemarkService.prototype = {
if (livemarksMap.has(guid)) {
let livemark = livemarksMap.get(guid);
livemark.terminate();
livemarksMap.delete(guid);
}
});
},
+ skipDescendantsOnItemRemoval: false,
+ skipTags: true,
+
// nsINavHistoryObserver
onPageChanged() {},
onTitleChanged() {},
onDeleteVisits() {},
onClearHistory() {
this._promiseLivemarksMap().then(livemarksMap => {