Bug 1410940 - Make PlacesController#paste and PlacesControllerDragHelper#onDrop more similar to each other. r?mak
MozReview-Commit-ID: EEFizNPmKpr
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -1330,29 +1330,29 @@ PlacesController.prototype = {
// source, otherwise report an error and fallback to a copy.
if (!doCopy &&
!PlacesControllerDragHelper.canMoveUnwrappedNode(item)) {
Components.utils.reportError("Tried to move an unmovable " +
"Places node, reverting to a copy operation.");
doCopy = true;
}
- transactionData.push([item, type, parent, insertionIndex, doCopy]);
+ transactionData.push(PlacesUIUtils.getTransactionForData(
+ item, type, parent, insertionIndex, doCopy));
// Adjust index to make sure items are pasted in the correct
// position. If index is DEFAULT_INDEX, items are just appended.
if (insertionIndex != PlacesUtils.bookmarks.DEFAULT_INDEX)
insertionIndex++;
}
await PlacesUIUtils.batchUpdatesForNode(this._view.result, transactionData.length, async () => {
await PlacesTransactions.batch(async () => {
- for (let item of transactionData) {
- let guid = await PlacesUIUtils.getTransactionForData(
- ...item).transact();
+ for (let transaction of transactionData) {
+ let guid = await transaction.transact();
itemsToSelect.push(await PlacesUtils.promiseItemId(guid));
}
});
});
}
} else {
let transactions = [];
let insertionIndex = await ip.getIndex();
@@ -1671,79 +1671,103 @@ var PlacesControllerDragHelper = {
let uri = data.linkedBrowser.currentURI;
let spec = uri ? uri.spec : "about:blank";
nodes = [{ uri: spec,
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();
+ if (PlacesUIUtils.useAsyncTransactions) {
+ // If dragging over a tag container we should tag the item.
+ if (insertionPoint.isTag) {
+ let urls = nodes.filter(item => "uri" in item).map(item => item.uri);
+ transactions.push(PlacesTransactions.Tag({ urls, tag: tagName }));
+ } else {
+ for (let unwrapped of nodes) {
+ let index = await insertionPoint.getIndex();
- 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);
+ 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);
- // 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;
+ // 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 (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 {
+ }
+
+ // If this is not a copy, check for safety that we can move the
+ // source, otherwise report an error and fallback to a copy.
+ if (!doCopy && !PlacesControllerDragHelper.canMoveUnwrappedNode(unwrapped)) {
+ Components.utils.reportError("Tried to move an unmovable Places " +
+ "node, reverting to a copy operation.");
+ doCopy = true;
+ }
+ transactions.push(
+ PlacesUIUtils.getTransactionForData(unwrapped,
+ flavor,
+ parentGuid,
+ index,
+ doCopy));
+ }
+ }
+ } else {
+ for (let unwrapped of nodes) {
+ let index = await insertionPoint.getIndex();
+
+ 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);
+
+ // 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) {
// 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 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)
- transactions.push(PlacesTransactions.Tag({ uri, tag: tagName }));
- else
+ // If dragging over a tag container we should tag the item.
+ // eslint-disable-next-line no-lonely-if
+ if (insertionPoint.isTag) {
+ let uri = NetUtil.newURI(unwrapped.uri);
+ let tagItemId = insertionPoint.itemId;
transactions.push(new PlacesTagURITransaction(uri, [tagItemId]));
- } else {
- // If this is not a copy, check for safety that we can move the
- // source, otherwise report an error and fallback to a copy.
- if (!doCopy && !PlacesControllerDragHelper.canMoveUnwrappedNode(unwrapped)) {
- Components.utils.reportError("Tried to move an unmovable Places " +
- "node, reverting to a copy operation.");
- doCopy = true;
- }
- if (PlacesUIUtils.useAsyncTransactions) {
- transactions.push(
- PlacesUIUtils.getTransactionForData(unwrapped,
- flavor,
- parentGuid,
- index,
- doCopy));
} else {
+ // If this is not a copy, check for safety that we can move the
+ // source, otherwise report an error and fallback to a copy.
+ if (!doCopy && !PlacesControllerDragHelper.canMoveUnwrappedNode(unwrapped)) {
+ Components.utils.reportError("Tried to move an unmovable Places " +
+ "node, reverting to a copy operation.");
+ doCopy = true;
+ }
transactions.push(PlacesUIUtils.makeTransaction(unwrapped,
flavor, insertionPoint.itemId,
index, doCopy));
}
}
}
}
// Check if we actually have something to add, if we don't it probably wasn't
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -22,16 +22,17 @@ support-files =
[browser_bookmarkProperties_addLivemark.js]
[browser_bookmarkProperties_bookmarkAllTabs.js]
[browser_bookmarkProperties_cancel.js]
[browser_bookmarkProperties_editTagContainer.js]
[browser_bookmarkProperties_readOnlyRoot.js]
[browser_bookmarksProperties.js]
[browser_check_correct_controllers.js]
[browser_click_bookmarks_on_toolbar.js]
+[browser_controller_onDrop_tagFolder.js]
[browser_controller_onDrop.js]
[browser_copy_folder_tree.js]
[browser_copy_query_without_tree.js]
subsuite = clipboard
[browser_cutting_bookmarks.js]
subsuite = clipboard
[browser_drag_bookmarks_on_toolbar.js]
[browser_forgetthissite_single.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_controller_onDrop_tagFolder.js
@@ -0,0 +1,112 @@
+/* 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";
+
+/* global sinon */
+Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js");
+
+const sandbox = sinon.sandbox.create();
+const TAG_NAME = "testTag";
+
+var bookmarks;
+var bookmarkId;
+
+add_task(async function setup() {
+ registerCleanupFunction(async function() {
+ sandbox.restore();
+ delete window.sinon;
+ await PlacesUtils.bookmarks.eraseEverything();
+ await PlacesTestUtils.clearHistory();
+ });
+
+ sandbox.stub(PlacesTransactions, "batch");
+ sandbox.stub(PlacesTransactions, "Tag");
+
+ bookmarks = await PlacesUtils.bookmarks.insertTree({
+ guid: PlacesUtils.bookmarks.unfiledGuid,
+ children: [{
+ title: "bm1",
+ url: "http://example1.com"
+ }, {
+ title: "bm2",
+ url: "http://example2.com",
+ tags: [TAG_NAME]
+ }]
+ });
+ bookmarkId = await PlacesUtils.promiseItemId(bookmarks[0].guid);
+});
+
+async function run_drag_test(startBookmarkIndex, newParentGuid) {
+ 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.
+ PlacesTransactions.Tag.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({
+ isTag: true,
+ tagName: TAG_NAME,
+ orientation: Ci.nsITreeView.DROP_ON
+ });
+
+ let bookmarkWithId = JSON.stringify(Object.assign({
+ id: bookmarkId,
+ itemGuid: dragBookmark.guid,
+ parent: PlacesUtils.unfiledBookmarksFolderId,
+ uri: dragBookmark.url
+ }, dragBookmark));
+
+ let dt = {
+ dropEffect: "move",
+ mozCursor: "auto",
+ mozItemCount: 1,
+ types: [ PlacesUtils.TYPE_X_MOZ_PLACE ],
+ mozTypesAt(i) {
+ return this.types;
+ },
+ mozGetDataAt(i) {
+ return bookmarkWithId;
+ }
+ };
+
+ await PlacesControllerDragHelper.onDrop(ip, dt);
+
+ Assert.ok(PlacesTransactions.Tag.calledOnce,
+ "Should have called getTransactionForData at least once.");
+
+ let arg = PlacesTransactions.Tag.args[0][0];
+
+ Assert.equal(arg.urls.length, 1,
+ "Should have called PlacesTransactions.Tag with an array of one url");
+ Assert.equal(arg.urls[0], dragBookmark.url,
+ "Should have called PlacesTransactions.Tag with the correct url");
+ Assert.equal(arg.tag, TAG_NAME,
+ "Should have called PlacesTransactions.Tag with the correct tag name");
+ });
+}
+
+add_task(async function test_simple_drop_and_tag() {
+ // 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, PlacesUtils.bookmarks.tagGuid);
+});