commit 1a49018bdcaa8c1704374ad568946a6ae89df231
parent 76bc61e8827d43b44c719c6956a16e99d97f8fbe
Author: Dan Stillman <dstillman@zotero.org>
Date: Fri, 3 Feb 2017 00:04:05 -0500
Fix moving items between collections
`mozSourceNode` seems to no longer be set in `dataTransfer` objects
during drags, so we now store it in `Zotero.DragDrop`.
Diffstat:
5 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -2518,6 +2518,12 @@ Zotero.ItemTreeView.prototype.onDragStart = function (event) {
var itemIDs = this.getSelectedItems(true);
event.dataTransfer.setData("zotero/item", itemIDs);
+ // dataTransfer.mozSourceNode doesn't seem to be properly set anymore (tested in 50), so store
+ // target separately
+ if (!event.dataTransfer.mozSourceNode) {
+ Zotero.debug("mozSourceNode not set -- storing source node");
+ Zotero.DragDrop.currentSourceNode = event.target;
+ }
var items = Zotero.Items.get(itemIDs);
@@ -2609,6 +2615,12 @@ Zotero.ItemTreeView.prototype.onDragStart = function (event) {
}
};
+Zotero.ItemTreeView.prototype.onDragEnd = function (event) {
+ setTimeout(function () {
+ Zotero.DragDrop.currentDragSource = null;
+ });
+}
+
// Implements nsIFlavorDataProvider for dragging attachment files to OS
//
diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js
@@ -310,10 +310,10 @@ Zotero.LibraryTreeView.prototype = {
}
if (event.dataTransfer.getData("zotero/item")) {
- var sourceCollectionTreeRow = Zotero.DragDrop.getDragSource();
+ var sourceCollectionTreeRow = Zotero.DragDrop.getDragSource(event.dataTransfer);
if (sourceCollectionTreeRow) {
if (this.type == 'collection') {
- var targetCollectionTreeRow = Zotero.DragDrop.getDragTarget();
+ var targetCollectionTreeRow = Zotero.DragDrop.getDragTarget(event);
}
else if (this.type == 'item') {
var targetCollectionTreeRow = this.collectionTreeRow;
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
@@ -2312,6 +2312,7 @@ Zotero.VersionHeader = {
Zotero.DragDrop = {
currentEvent: null,
currentOrientation: 0,
+ currentSourceNode: null,
getDataFromDataTransfer: function (dataTransfer, firstOnly) {
var dt = dataTransfer;
@@ -2374,7 +2375,7 @@ Zotero.DragDrop = {
// For items, the drag source is the CollectionTreeRow of the parent window
// of the source tree
if (dataTransfer.types.contains("zotero/item")) {
- var sourceNode = dataTransfer.mozSourceNode;
+ let sourceNode = dataTransfer.mozSourceNode || this.currentSourceNode;
if (!sourceNode || sourceNode.tagName != 'treechildren'
|| sourceNode.parentElement.id != 'zotero-items-tree') {
return false;
@@ -2385,9 +2386,8 @@ Zotero.DragDrop = {
}
return win.ZoteroPane.collectionsView.selectedTreeRow;
}
- else {
- return false;
- }
+
+ return false;
},
diff --git a/chrome/content/zotero/zoteroPane.xul b/chrome/content/zotero/zoteroPane.xul
@@ -554,7 +554,8 @@
<treechildren ondragstart="ZoteroPane_Local.itemsView.onDragStart(event)"
ondragenter="return ZoteroPane_Local.itemsView.onDragEnter(event)"
ondragover="return ZoteroPane_Local.itemsView.onDragOver(event)"
- ondrop="return ZoteroPane_Local.itemsView.onDrop(event)"/>
+ ondrop="return ZoteroPane_Local.itemsView.onDrop(event)"
+ ondragend="ZoteroPane_Local.itemsView.onDragEnd(event)"/>
</tree>
<!-- Label for displaying messages when items pane is hidden
diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js
@@ -450,7 +450,7 @@ describe("Zotero.CollectionTreeView", function() {
* value returned after the drag. Otherwise, an 'add' event will be waited for, and
* an object with 'ids' and 'extraData' will be returned.
*/
- var drop = Zotero.Promise.coroutine(function* (objectType, targetRow, ids, promise) {
+ var drop = Zotero.Promise.coroutine(function* (objectType, targetRow, ids, promise, action = 'copy') {
if (typeof targetRow == 'string') {
var row = cv.getRowIndexByID(targetRow);
var orient = 0;
@@ -465,9 +465,9 @@ describe("Zotero.CollectionTreeView", function() {
promise = waitForNotifierEvent("add", objectType);
}
yield cv.drop(row, orient, {
- dropEffect: 'copy',
- effectAllowed: 'copy',
- mozSourceNode: win.document.getElementById(`zotero-${objectType}s-tree`),
+ dropEffect: action,
+ effectAllowed: action,
+ mozSourceNode: win.document.getElementById(`zotero-${objectType}s-tree`).treeBoxObject.treeBody,
types: {
contains: function (type) {
return type == `zotero/${objectType}`;
@@ -546,6 +546,41 @@ describe("Zotero.CollectionTreeView", function() {
assert.equal(treeRow.ref.id, item.id);
})
+ it("should move an item from one collection to another", function* () {
+ var collection1 = yield createDataObject('collection');
+ yield waitForItemsLoad(win);
+ var collection2 = yield createDataObject('collection', false, { skipSelect: true });
+ var item = yield createDataObject('item', { collections: [collection1.id] });
+
+ // Add observer to wait for collection add
+ var deferred = Zotero.Promise.defer();
+ var observerID = Zotero.Notifier.registerObserver({
+ notify: function (event, type, ids, extraData) {
+ if (type == 'collection-item' && event == 'add'
+ && ids[0] == collection2.id + "-" + item.id) {
+ setTimeout(function () {
+ deferred.resolve();
+ });
+ }
+ }
+ }, 'collection-item', 'test');
+
+ yield drop('item', 'C' + collection2.id, [item.id], deferred.promise, 'move');
+
+ Zotero.Notifier.unregisterObserver(observerID);
+
+ // Source collection should be empty
+ assert.equal(zp.itemsView.rowCount, 0);
+
+ yield cv.selectCollection(collection2.id);
+ yield waitForItemsLoad(win);
+
+ // Target collection should have item
+ assert.equal(zp.itemsView.rowCount, 1);
+ var treeRow = zp.itemsView.getRow(0);
+ assert.equal(treeRow.ref.id, item.id);
+ });
+
it("should copy an item with an attachment to a group", function* () {
var group = yield createGroup();
