Fix dragging of URLs into items list

chrome/content/zotero/xpcom/attachments.js

@@ -39,6 +39,7 @@ Zotero.Attachments = new function(){
 	 * @param {Integer} [options.libraryID]
 	 * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to
 	 * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to
+	 * @param {Object} [options.saveOptions] - Options to pass to Zotero.Item::save()
 	 * @return {Promise<Zotero.Item>}
 	 */
 	this.importFromFile = Zotero.Promise.coroutine(function* (options) {
@@ -48,6 +49,7 @@ Zotero.Attachments = new function(){
 		var file = Zotero.File.pathToFile(options.file);
 		var parentItemID = options.parentItemID;
 		var collections = options.collections;
+		var saveOptions = options.saveOptions;
 		
 		var newName = Zotero.File.getValidFileName(file.leafName);
 		
@@ -76,7 +78,7 @@ Zotero.Attachments = new function(){
 			if (collections) {
 				attachmentItem.setCollections(collections);
 			}
-			yield attachmentItem.save();
+			yield attachmentItem.save(saveOptions);
 			
 			// Create directory for attachment files within storage directory
 			destDir = yield this.createDirectoryForItem(attachmentItem);
@@ -92,7 +94,7 @@ Zotero.Attachments = new function(){
 			
 			attachmentItem.attachmentContentType = contentType;
 			attachmentItem.attachmentPath = newFile.path;
-			yield attachmentItem.save();
+			yield attachmentItem.save(saveOptions);
 		}.bind(this))
 		.then(function () {
 			return _postProcessFile(attachmentItem, newFile, contentType);
@@ -122,6 +124,7 @@ Zotero.Attachments = new function(){
 	 * @param {nsIFile} [options.file] - File to link to
 	 * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to
 	 * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to
+	 * @param {Object} [options.saveOptions] - Options to pass to Zotero.Item::save()
 	 * @return {Promise<Zotero.Item>}
 	 */
 	this.linkFromFile = Zotero.Promise.coroutine(function* (options) {
@@ -130,6 +133,7 @@ Zotero.Attachments = new function(){
 		var file = options.file;
 		var parentItemID = options.parentItemID;
 		var collections = options.collections;
+		var saveOptions = options.saveOptions;
 		
 		if (parentItemID && collections) {
 			throw new Error("parentItemID and collections cannot both be provided");
@@ -138,12 +142,13 @@ Zotero.Attachments = new function(){
 		var title = file.leafName;
 		var contentType = yield Zotero.MIME.getMIMETypeFromFile(file);
 		var item = yield _addToDB({
-			file: file,
+			file,
-			title: title,
+			title,
 			linkMode: this.LINK_MODE_LINKED_FILE,
-			contentType: contentType,
+			contentType,
-			parentItemID: parentItemID,
+			parentItemID,
-			collections: collections
+			collections,
+			saveOptions
 		});
 		yield _postProcessFile(item, file, contentType);
 		return item;
@@ -222,7 +227,7 @@ Zotero.Attachments = new function(){
 	
 	/**
 	 * @param {Object} options - 'libraryID', 'url', 'parentItemID', 'collections', 'title',
-	 *                           'fileBaseName', 'contentType', 'cookieSandbox'
+	 *                           'fileBaseName', 'contentType', 'cookieSandbox', 'saveOptions'
 	 * @return {Promise<Zotero.Item>} - A promise for the created attachment item
 	 */
 	this.importFromURL = Zotero.Promise.coroutine(function* (options) {
@@ -234,6 +239,7 @@ Zotero.Attachments = new function(){
 		var fileBaseName = options.fileBaseName;
 		var contentType = options.contentType;
 		var cookieSandbox = options.cookieSandbox;
+		var saveOptions = options.saveOptions;
 		
 		Zotero.debug('Importing attachment from URL ' + url);
 		
@@ -264,11 +270,12 @@ Zotero.Attachments = new function(){
 						}
 					}
 					return Zotero.Attachments.importFromDocument({
-						libraryID: libraryID,
+						libraryID,
 						document: browser.contentDocument,
-						parentItemID: parentItemID,
+						parentItemID,
-						title: title,
+						title,
-						collections: collections
+						collections,
+						saveOptions
 					})
 					.then(function (attachmentItem) {
 						Zotero.Browser.deleteHiddenBrowser(browser);
@@ -352,7 +359,7 @@ Zotero.Attachments = new function(){
 				if (collections) {
 					attachmentItem.setCollections(collections);
 				}
-				var itemID = yield attachmentItem.save();
+				var itemID = yield attachmentItem.save(saveOptions);
 				
 				// Create a new folder for this item in the storage directory
 				destDir = this.getStorageDirectory(attachmentItem);
@@ -362,7 +369,7 @@ Zotero.Attachments = new function(){
 				
 				// Refetch item to update path
 				attachmentItem.attachmentPath = destFile.path;
-				yield attachmentItem.save();
+				yield attachmentItem.save(saveOptions);
 			}.bind(this))
 			.catch(function (e) {
 				Zotero.debug(e, 1);
@@ -1223,7 +1230,8 @@ Zotero.Attachments = new function(){
 	/**
 	 * Create a new item of type 'attachment' and add to the itemAttachments table
 	 *
-	 * @param {Object} options - 'file', 'url', 'title', 'linkMode', 'contentType', 'charsetID', 'parentItemID'
+	 * @param {Object} options - 'file', 'url', 'title', 'linkMode', 'contentType', 'charsetID',
+	 *     'parentItemID', 'saveOptions'
 	 * @return {Promise<Zotero.Item>} - A promise for the new attachment
 	 */
 	function _addToDB(options) {
@@ -1235,6 +1243,7 @@ Zotero.Attachments = new function(){
 		var charset = options.charset;
 		var parentItemID = options.parentItemID;
 		var collections = options.collections;
+		var saveOptions = options.saveOptions;
 		
 		return Zotero.DB.executeTransaction(function* () {
 			var attachmentItem = new Zotero.Item('attachment');
@@ -1264,7 +1273,7 @@ Zotero.Attachments = new function(){
 			if (collections) {
 				attachmentItem.setCollections(collections);
 			}
-			yield attachmentItem.save();
+			yield attachmentItem.save(saveOptions);
 			
 			return attachmentItem;
 		}.bind(this));

chrome/content/zotero/xpcom/itemTreeView.js

@@ -3048,14 +3048,13 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 			var parentCollectionID = collectionTreeRow.ref.id;
 		}
 		
-		var unlock = Zotero.Notifier.begin(true);
+		var notifierQueue = new Zotero.Notifier.Queue;
 		try {
 			for (var i=0; i<data.length; i++) {
 				var file = data[i];
 				
 				if (dataType == 'text/x-moz-url') {
 					var url = data[i];
-					
 					if (url.indexOf('file:///') == 0) {
 						var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
 								   .getService(Components.interfaces.nsIWindowMediator);
@@ -3085,7 +3084,14 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 								win.ZoteroPane.displayCannotEditLibraryFilesMessage();
 								return;
 							}
-							Zotero.Attachments.importFromURL(url, parentItemID, false, false, null, null, targetLibraryID);
+							yield Zotero.Attachments.importFromURL({
+								libraryID: targetLibraryID,
+								url,
+								parentItemID,
+								saveOptions: {
+									notifierQueue
+								}
+							});
 						}
 						else {
 							var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
@@ -3101,9 +3107,12 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 				
 				if (dropEffect == 'link') {
 					yield Zotero.Attachments.linkFromFile({
-						file: file,
+						file,
-						parentItemID: parentItemID,
+						parentItemID,
-						collections: parentCollectionID ? [parentCollectionID] : undefined
+						collections: parentCollectionID ? [parentCollectionID] : undefined,
+						saveOptions: {
+							notifierQueue
+						}
 					});
 				}
 				else {
@@ -3115,10 +3124,13 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 						continue;
 					}
 					yield Zotero.Attachments.importFromFile({
-						file: file,
+						file,
 						libraryID: targetLibraryID,
-						parentItemID: parentItemID,
+						parentItemID,
-						collections: parentCollectionID ? [parentCollectionID] : undefined
+						collections: parentCollectionID ? [parentCollectionID] : undefined,
+						saveOptions: {
+							notifierQueue
+						}
 					});
 					// If moving, delete original file
 					if (dragData.dropEffect == 'move') {
@@ -3133,7 +3145,7 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 			}
 		}
 		finally {
-			Zotero.Notifier.commit(unlock);
+			yield Zotero.Notifier.commit(notifierQueue);
 		}
 	}
 });

chrome/content/zotero/zoteroPane.js

@@ -3726,13 +3726,13 @@ var ZoteroPane = new function()
 						var collectionID = false;
 					}
 					
-					// TODO: Update for async DB
+					let attachmentItem = yield Zotero.Attachments.importFromURL({
-					var attachmentItem = Zotero.Attachments.importFromURL(url, false,
+						libraryID,
-						false, false, collectionID, mimeType, libraryID,
+						url,
-						function(attachmentItem) {
+						collections: collectionID ? [collectionID] : undefined,
-							self.selectItem(attachmentItem.id);
+						contentType: mimeType
-						});
+					});
-					
+					this.selectItem(attachmentItem.id)
 					return;
 				}
 			}
@@ -3752,7 +3752,11 @@ var ZoteroPane = new function()
 					//Zotero.Attachments.linkFromURL(doc, item.id);
 				}
 				else if (filesEditable) {
-					var attachmentItem = Zotero.Attachments.importFromURL(url, item.id, false, false, false, mimeType);
+					var attachmentItem = yield Zotero.Attachments.importFromURL({
+						url,
+						parentItemID: item.id,
+						contentType: mimeType
+					});
 					if (attachmentItem) {
 						item.setField('title', attachmentItem.getField('title'));
 						item.setField('url', attachmentItem.getField('url'));

test/tests/itemTreeViewTest.js

@@ -307,6 +307,30 @@ describe("Zotero.ItemTreeView", function() {
 	})
 	
 	describe("#drop()", function () {
+		var httpd;
+		var port = 16213;
+		var baseURL = `http://localhost:${port}/`;
+		var pdfFilename = "test.pdf";
+		var pdfURL = baseURL + pdfFilename;
+		var pdfPath;
+		
+		// Serve a PDF to test URL dragging
+		before(function () {
+			Components.utils.import("resource://zotero-unit/httpd.js");
+			httpd = new HttpServer();
+			httpd.start(port);
+			var file = getTestDataDirectory();
+			file.append(pdfFilename);
+			pdfPath = file.path;
+			httpd.registerFile("/" + pdfFilename, file);
+		});
+		
+		after(function* () {
+			var defer = new Zotero.Promise.defer();
+			httpd.stop(() => defer.resolve());
+			yield defer.promise;
+		});
+		
 		it("should move a child item from one item to another", function* () {
 			var collection = yield createDataObject('collection');
 			yield waitForItemsLoad(win);
@@ -417,6 +441,68 @@ describe("Zotero.ItemTreeView", function() {
 				(yield Zotero.File.getBinaryContentsAsync(path)),
 				(yield Zotero.File.getBinaryContentsAsync(file))
 			);
-		})
+		});
+		
+		it("should create a top-level attachment when a URL is dragged", function* () {
+			var deferred = Zotero.Promise.defer();
+			itemsView.addEventListener('select', () => deferred.resolve());
+			
+			itemsView.drop(0, -1, {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				types: {
+					contains: function (type) {
+						return type == 'text/x-moz-url';
+					}
+				},
+				getData: function (type) {
+					if (type == 'text/x-moz-url') {
+						return pdfURL;
+					}
+				},
+				mozItemCount: 1,
+			})
+			
+			yield deferred.promise;
+			var item = itemsView.getSelectedItems()[0];
+			assert.equal(item.getField('url'), pdfURL);
+			assert.equal(
+				(yield Zotero.File.getBinaryContentsAsync(yield item.getFilePathAsync())),
+				(yield Zotero.File.getBinaryContentsAsync(pdfPath))
+			);
+		});
+		
+		it("should create a child attachment when a URL is dragged", function* () {
+			var view = zp.itemsView;
+			var parentItem = yield createDataObject('item');
+			var parentRow = view.getRowIndexByID(parentItem.id);
+			
+			var promise = waitForItemEvent('add');
+			
+			itemsView.drop(parentRow, 0, {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				types: {
+					contains: function (type) {
+						return type == 'text/x-moz-url';
+					}
+				},
+				getData: function (type) {
+					if (type == 'text/x-moz-url') {
+						return pdfURL;
+					}
+				},
+				mozItemCount: 1,
+			})
+			
+			var itemIDs = yield promise;
+			var item = Zotero.Items.get(itemIDs[0]);
+			assert.equal(item.parentItemID, parentItem.id);
+			assert.equal(item.getField('url'), pdfURL);
+			assert.equal(
+				(yield Zotero.File.getBinaryContentsAsync(yield item.getFilePathAsync())),
+				(yield Zotero.File.getBinaryContentsAsync(pdfPath))
+			);
+		});
 	});
 })