Bug 1439697 - Always send the item guid in serialized copy data, and make the copy algorithm clearer as to what items use which methods. r?mak
MozReview-Commit-ID: 4dwJB2x2qyp
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -1160,21 +1160,17 @@ async function getTransactionsForTransfe
insertionParentGuid, doMove) {
let canMove = true;
for (let item of items) {
if (!PlacesUIUtils.SUPPORTED_FLAVORS.includes(item.type)) {
throw new Error(`Unsupported '${item.type}' data type`);
}
// Work out if this is data from the same app session we're running in.
- if ("instanceId" in item && item.instanceId == PlacesUtils.instanceId) {
- if ("itemGuid" in item && !PlacesUIUtils.PLACES_FLAVORS.includes(item.type)) {
- throw new Error(`itemGuid unexpectedly set on ${item.type} data`);
- }
- } else {
+ if (!("instanceId" in item) || item.instanceId != PlacesUtils.instanceId) {
if (item.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER) {
throw new Error("Can't copy a container from a legacy-transactions build");
}
// We can never move from an external copy.
canMove = false;
}
@@ -1253,22 +1249,32 @@ async function getTransactionsForMove(it
*/
async function getTransactionsForCopy(items, insertionIndex,
insertionParentGuid) {
let transactions = [];
let index = insertionIndex;
for (let item of items) {
let transaction;
+ let guid = item.itemGuid;
- if ("itemGuid" in item && "instanceId" in item &&
- item.instanceId == PlacesUtils.instanceId) {
+ if (PlacesUIUtils.PLACES_FLAVORS.includes(item.type) &&
+ // For anything that is comming from within this session, we do a
+ // direct copy, otherwise we fallback and form a new item below.
+ "instanceId" in item && item.instanceId == PlacesUtils.instanceId &&
+ // If the Item doesn't have a guid, this could be a virtual tag query or
+ // other item, so fallback to inserting a new bookmark with the URI.
+ guid &&
+ // For virtual root items, we fallback to creating a new bookmark, as
+ // we want a shortcut to be created, not a full tree copy.
+ !PlacesUtils.bookmarks.isVirtualRootItem(guid) &&
+ !PlacesUtils.isVirtualLeftPaneItem(guid)) {
transaction = PlacesTransactions.Copy({
excludingAnnotation: "Places/SmartBookmark",
- guid: item.itemGuid,
+ guid,
newIndex: index,
newParentGuid: insertionParentGuid,
});
} else if (item.type == PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR) {
transaction = PlacesTransactions.NewSeparator({
index,
parentGuid: insertionParentGuid,
});
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -102,32 +102,33 @@ async function notifyKeywordChange(url,
* An nsINavHistoryResultNode
* @param aIsLivemark
* Whether the node represents a livemark.
*/
function serializeNode(aNode, aIsLivemark) {
let data = {};
data.title = aNode.title;
+ // The id is no longer used for copying within the same instance/session of
+ // Firefox as of at least 61. However, we keep the id for now to maintain
+ // backwards compat of drag and drop with older Firefox versions.
data.id = aNode.itemId;
+ data.itemGuid = aNode.bookmarkGuid;
data.livemark = aIsLivemark;
// Add an instanceId so we can tell which instance of an FF session the data
// is coming from.
data.instanceId = PlacesUtils.instanceId;
let guid = aNode.bookmarkGuid;
let grandParentId;
// Some nodes, e.g. the unfiled/menu/toolbar ones can have a virtual guid, so
// we ignore any that are a folder shortcut. These will be handled below.
if (guid && !PlacesUtils.bookmarks.isVirtualRootItem(guid) &&
!PlacesUtils.isVirtualLeftPaneItem(guid)) {
- // TODO: Really guid should be set on everything, however currently this upsets
- // the drag 'n' drop / cut/copy/paste operations.
- data.itemGuid = guid;
if (aNode.parent) {
data.parent = aNode.parent.itemId;
data.parentGuid = aNode.parent.bookmarkGuid;
}
let grandParent = aNode.parent && aNode.parent.parent;
if (grandParent) {
grandParentId = grandParent.itemId;