--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -1629,38 +1629,49 @@ var PlacesControllerDragHelper = {
title: data.label,
type: PlacesUtils.TYPE_X_MOZ_URL}];
} else
throw new Error("bogus data was passed as a tab");
for (let unwrapped of nodes) {
let index = await insertionPoint.getIndex();
- // Adjust insertion index to prevent reversal of dragged items. When you
- // drag multiple elts upward: need to increment index or each successive
- // elt will be inserted at the same index, each above the previous.
if (index != -1 && unwrapped.itemGuid) {
// Note: we use the parent from the existing bookmark as the sidebar
// gives us an unwrapped.parent that is actually a query and not the real
// parent.
let existingBookmark = await PlacesUtils.bookmarks.fetch(unwrapped.itemGuid);
- let dragginUp = parentGuid == existingBookmark.parentGuid &&
- index < existingBookmark.index;
- if (dragginUp) {
- index += movedCount++;
- } else if (PlacesUIUtils.useAsyncTransactions) {
- if (index == existingBookmark.index) {
- // We're moving to the same index, so there's nothing for us to do.
- continue;
+ // If we're dropping on the same folder, then we may need to adjust
+ // the index to insert at the correct place.
+ if (existingBookmark && parentGuid == existingBookmark.parentGuid) {
+ if (PlacesUIUtils.useAsyncTransactions) {
+ if (index < existingBookmark.index) {
+ // When you drag multiple elts upward: need to increment index or
+ // each successive elt will be inserted at the same index, each
+ // above the previous.
+ index += movedCount++;
+ } else if (index > existingBookmark.index) {
+ // If we're dragging down, we need to go one lower to insert at
+ // the real point as moving the element changes the index of
+ // everything below by 1.
+ index--;
+ } else {
+ // This isn't moving so we skip it.
+ continue;
+ }
+ } else {
+ // Sync Transactions. Adjust insertion index to prevent reversal
+ // of dragged items. When you drag multiple elts upward: need to
+ // increment index or each successive elt will be inserted at the
+ // same index, each above the previous.
+ if (index < existingBookmark.index) { // eslint-disable-line no-lonely-if
+ index += movedCount++;
+ }
}
- // If we're dragging down, we need to go one lower to insert at
- // the real point as moving the element changes the index of
- // everything below by 1.
- index--;
}
}
// If dragging over a tag container we should tag the item.
if (insertionPoint.isTag) {
let uri = NetUtil.newURI(unwrapped.uri);
let tagItemId = insertionPoint.itemId;
if (PlacesUIUtils.useAsyncTransactions)
--- a/browser/components/places/tests/browser/browser_controller_onDrop.js
+++ b/browser/components/places/tests/browser/browser_controller_onDrop.js
@@ -29,49 +29,66 @@ add_task(async function setup() {
title: "bm1",
url: "http://example1.com"
}, {
title: "bm2",
url: "http://example2.com"
}, {
title: "bm3",
url: "http://example3.com"
+ }, {
+ title: "folder1",
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ children: [{
+ title: "bm4",
+ url: "http://example1.com"
+ }, {
+ title: "bm5",
+ url: "http://example2.com"
+ }, {
+ title: "bm6",
+ url: "http://example3.com"
+ }]
}]
});
bookmarkIds = await PlacesUtils.promiseManyItemIds([
bookmarks[0].guid,
bookmarks[1].guid,
bookmarks[2].guid,
]);
});
-async function run_drag_test(startBookmarkIndex, insertionIndex,
- realInsertionIndex, expectTransactionCreated = true) {
+async function run_drag_test(startBookmarkIndex, insertionIndex, newParentGuid,
+ expectedInsertionIndex, expectTransactionCreated = true) {
if (!PlacesUIUtils.useAsyncTransactions) {
Assert.ok(true, "Skipping test as async transactions are turned off");
return;
}
+ if (!newParentGuid) {
+ newParentGuid = PlacesUtils.bookmarks.unfiledGuid;
+ }
+
// Reset the stubs so that previous test runs don't count against us.
PlacesUIUtils.getTransactionForData.reset();
PlacesTransactions.batch.reset();
let dragBookmark = bookmarks[startBookmarkIndex];
await withSidebarTree("bookmarks", async (tree) => {
tree.selectItems([PlacesUtils.unfiledBookmarksFolderId]);
PlacesUtils.asContainer(tree.selectedNode).containerOpen = true;
// Simulating a drag-drop with a tree view turns out to be really difficult
// as you can't get a node for the source/target. Hence, we fake the
// insertion point and drag data and call the function direct.
let ip = new InsertionPoint({
parentId: await PlacesUtils.promiseItemId(PlacesUtils.bookmarks.unfiledGuid),
- parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ parentGuid: newParentGuid,
index: insertionIndex,
orientation: Ci.nsITreeView.DROP_ON
});
let bookmarkWithId = JSON.stringify(Object.assign({
id: bookmarkIds.get(dragBookmark.guid),
itemGuid: dragBookmark.guid,
parent: PlacesUtils.unfiledBookmarksFolderId,
@@ -102,36 +119,42 @@ async function run_drag_test(startBookma
"Should have called getTransactionForData at least once.");
let args = PlacesUIUtils.getTransactionForData.args[0];
Assert.deepEqual(args[0], JSON.parse(bookmarkWithId),
"Should have called getTransactionForData with the correct unwrapped bookmark");
Assert.equal(args[1], PlacesUtils.TYPE_X_MOZ_PLACE,
"Should have called getTransactionForData with the correct flavor");
- Assert.equal(args[2], PlacesUtils.bookmarks.unfiledGuid,
+ Assert.equal(args[2], newParentGuid,
"Should have called getTransactionForData with the correct parent guid");
- Assert.equal(args[3], realInsertionIndex,
+ Assert.equal(args[3], expectedInsertionIndex,
"Should have called getTransactionForData with the correct index");
Assert.equal(args[4], false,
"Should have called getTransactionForData with a move");
});
}
add_task(async function test_simple_move_down() {
// When we move items down the list, we'll get a drag index that is one higher
// than where we actually want to insert to - as the item is being moved up,
// everything shifts down one. Hence the index to pass to the transaction should
// be one less than the supplied index.
- await run_drag_test(0, 2, 1);
+ await run_drag_test(0, 2, null, 1);
});
add_task(async function test_simple_move_up() {
// When we move items up the list, we want the matching index to be passed to
// the transaction as there's no changes below the item in the list.
- await run_drag_test(2, 0, 0);
+ await run_drag_test(2, 0, null, 0);
});
-add_task(async function test_simple_move_to_same() {
+add_task(async function test_simple_move_to_same_index() {
// If we move to the same index, then we don't expect any transactions to be
// created.
- await run_drag_test(1, 1, 1, false);
+ await run_drag_test(1, 1, null, 1, false);
});
+
+add_task(async function test_simple_move_different_folder() {
+ // When we move items to a different folder, the index should never change.
+ await run_drag_test(0, 2, bookmarks[3].guid, 2);
+ await run_drag_test(2, 0, bookmarks[3].guid, 0);
+});