Fix various cases of saving items to selected collection

This changes Zotero.Translate.Base.translate() to take an options object (in
order to take a 'collections' parameter, which is passed to the
Zotero.Translate.ItemSaver constructor). The old parameters are still supported
with a deprecation warning, and there may be other places that still need to be
updated.

chrome/content/zotero/browser.js

@@ -150,29 +150,34 @@ var Zotero_Browser = new function() {
 	 *
 	 * @param {String} [translator]
 	 * @param {Event} [event]
+	 * @return {Promise}
 	 */
-	this.scrapeThisPage = function (translator, event) {
+	this.scrapeThisPage = Zotero.Promise.coroutine(function* (translator, event) {
 		// Perform translation
 		var tab = _getTabObject(Zotero_Browser.tabbrowser.selectedBrowser);
 		var page = tab.getPageObject();
 		if(page.translators && page.translators.length) {
 			page.translate.setTranslator(translator || page.translators[0]);
-			Zotero_Browser.performTranslation(page.translate); // TODO: async
+			yield Zotero_Browser.performTranslation(page.translate);
 		}
 		else {
-			this.saveAsWebPage(
+			yield this.saveAsWebPage(
 				(event && event.shiftKey) ? !Zotero.Prefs.get('automaticSnapshots') : null
 			);
 		}
-	}
+	});
 	
-	// Keep in sync with cmd_zotero_newItemFromCurrentPage
+	/**
+	 * Keep in sync with cmd_zotero_newItemFromCurrentPage
+	 *
+	 * @return {Promise}
+	 */
 	this.saveAsWebPage = function (includeSnapshots) {
 		// DEBUG: Possible to just trigger command directly with event? Assigning it to the
 		// command property of the icon doesn't seem to work, and neither does goDoCommand()
 		// from chrome://global/content/globalOverlay.js. Getting the command by id and
 		// running doCommand() works but doesn't pass the event.
-		ZoteroPane.addItemFromPage('temporaryPDFHack', includeSnapshots);
+		return ZoteroPane.addItemFromPage('temporaryPDFHack', includeSnapshots);
 	}
 	
 	/*
@@ -655,6 +660,8 @@ var Zotero_Browser = new function() {
 		translate.clearHandlers("itemDone");
 		translate.clearHandlers("attachmentProgress");
 		
+		var deferred = Zotero.Promise.defer();
+		
 		translate.setHandler("done", function(obj, returnValue) {		
 			if(!returnValue) {
 				Zotero_Browser.progress.show();
@@ -669,6 +676,8 @@ var Zotero_Browser = new function() {
 				Zotero_Browser.progress.startCloseTimer();
 			}
 			Zotero_Browser.isScraping = false;
+			
+			deferred.resolve();
 		});
 		
 		translate.setHandler("itemDone", function(obj, dbItem, item) {
@@ -683,11 +692,6 @@ var Zotero_Browser = new function() {
 						Zotero.Utilities.determineAttachmentIcon(attachment),
 						attachment.title, itemProgress));
 			}
-			
-			// add item to collection, if one was specified
-			if(collection) {
-				collection.addItem(dbItem.id);
-			}
 		});
 		
 		translate.setHandler("attachmentProgress", function(obj, attachment, progress, error) {
@@ -702,7 +706,12 @@ var Zotero_Browser = new function() {
 			}
 		});
 		
-		translate.translate(libraryID);
+		translate.translate({
+			libraryID,
+			collections: collection ? [collection.id] : false
+		});
+		
+		return deferred.promise;
 	});
 	
 	

chrome/content/zotero/xpcom/itemTreeView.js

@@ -1600,7 +1600,7 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
 	var selected = this.getSelectedItems(true);
 	if (selected.length == 1 && selected[0] == id) {
 		Zotero.debug("Item " + id + " is already selected");
-		return;
+		return true;
 	}
 	
 	var row = this._rowMap[id];

chrome/content/zotero/xpcom/translation/translate.js

@@ -512,7 +512,11 @@ Zotero.Translate.Sandbox = {
 						var newCallback = function(selectedItems) {
 							callbackExecuted = true;
 							if(haveAsyncHandler) {
-								translate.translate(translate._libraryID, translate._saveAttachments, selectedItems);
+								translate.translate({
+									libraryID: translate._libraryID,
+									saveAttachments: translate._saveAttachments,
+									selectedItems
+								});
 							} else {
 								returnedItems = transferObject(selectedItems);
 							}
@@ -1182,16 +1186,24 @@ Zotero.Translate.Base.prototype = {
 	 * @returns {Promise}                                       Promise resolved with saved items
 	 *                                                          when translation complete
 	 */
-	"translate":function(libraryID, saveAttachments) {		// initialize properties specific to each translation
+	"translate": function (options = {}, ...args) {		// initialize properties specific to each translation
+		if (typeof options == 'number') {
+			Zotero.debug("Translate: translate() now takes an object -- update your code", 2);
+			options = {
+				libraryID: options,
+				saveAttachments: args[0],
+				selectedItems: args[1]
+			};
+		}
+		
 		if(!this.translator || !this.translator.length) {
-			var args = arguments;
 			Zotero.debug("Translate: translate called without specifying a translator. Running detection first.");
 			this.setHandler('translators', function(me, translators) {
 				if(!translators.length) {
 					me.complete(false, "Could not find an appropriate translator");
 				} else {
 					me.setTranslator(translators);
-					Zotero.Translate.Base.prototype.translate.apply(me, args);
+					Zotero.Translate.Base.prototype.translate.call(me, options);
 				}
 			});
 			this.getTranslators();
@@ -1200,8 +1212,9 @@ Zotero.Translate.Base.prototype = {
 		
 		this._currentState = "translate";
 		
-		this._libraryID = libraryID;
+		this._libraryID = options.libraryID;
-		this._saveAttachments = saveAttachments === undefined || saveAttachments;
+		this._collections = options.collections;
+		this._saveAttachments = options.saveAttachments === undefined || options.saveAttachments;
 		this._savingAttachments = [];
 		this._savingItems = 0;
 		this._waitingForSave = false;
@@ -1849,6 +1862,7 @@ Zotero.Translate.Web.prototype._getParameters = function() {
 Zotero.Translate.Web.prototype._prepareTranslation = function() {
 	this._itemSaver = new Zotero.Translate.ItemSaver({
 		"libraryID":this._libraryID,
+		"collections": this._collections,
 		"attachmentMode":Zotero.Translate.ItemSaver[(this._saveAttachments ? "ATTACHMENT_MODE_DOWNLOAD" : "ATTACHMENT_MODE_IGNORE")],
 		"forceTagType":1,
 		"cookieSandbox":this._cookieSandbox,
@@ -1860,9 +1874,17 @@ Zotero.Translate.Web.prototype._prepareTranslation = function() {
 /**
  * Overload translate to set selectedItems
  */
-Zotero.Translate.Web.prototype.translate = function(libraryID, saveAttachments, selectedItems) {
+Zotero.Translate.Web.prototype.translate = function (options = {}, ...args) {
-	this._selectedItems = selectedItems;
+	if (typeof options == 'number') {
-	return Zotero.Translate.Base.prototype.translate.apply(this, [libraryID, saveAttachments]);
+		Zotero.debug("Translate: translate() now takes an object -- update your code", 2);
+		options = {
+			libraryID: options,
+			saveAttachments: args[0],
+			selectedItems: args[1]
+		};
+	}
+	this._selectedItems = options.selectedItems;
+	return Zotero.Translate.Base.prototype.translate.call(this, options);
 }
 
 /**
@@ -2161,6 +2183,7 @@ Zotero.Translate.Import.prototype._prepareTranslation = function() {
 
 	this._itemSaver = new Zotero.Translate.ItemSaver({
 		"libraryID":this._libraryID,
+		"collections": this._collections,
 		"attachmentMode":Zotero.Translate.ItemSaver[(this._saveAttachments ? "ATTACHMENT_MODE_FILE" : "ATTACHMENT_MODE_IGNORE")],
 		"baseURI":baseURI
 	});
@@ -2411,7 +2434,10 @@ Zotero.Translate.Search.prototype.complete = function(returnValue, error) {
 		if(error) Zotero.debug(this._generateErrorString(error), 3);
 		if(this.translator.length > 1) {
 			this.translator.shift();
-			this.translate(this._libraryID, this._saveAttachments);
+			this.translate({
+				libraryID: this._libraryID,
+				saveAttachments: this._saveAttachments
+			});
 			return;
 		} else {
 			error = "No items returned from any translator";

chrome/content/zotero/xpcom/translation/translate_item.js

@@ -45,6 +45,8 @@ Zotero.Translate.ItemSaver = function(options) {
 		this._libraryID = options.libraryID;
 	}
 	
+	this._collections = options.collections || false;
+	
 	// If group filesEditable==false, don't save attachments
 	this.attachmentMode = Zotero.Libraries.isFilesEditable(this._libraryID) ? options.attachmentMode :
 	                      Zotero.Translate.ItemSaver.ATTACHMENT_MODE_IGNORE;
@@ -109,6 +111,10 @@ Zotero.Translate.ItemSaver.prototype = {
 							};
 							newItem.fromJSON(this._deleteIrrelevantFields(item));
 							
+							if (this._collections) {
+								newItem.setCollections(this._collections);
+							}
+							
 							// save item
 							myID = yield newItem.save();
 
@@ -596,6 +602,9 @@ Zotero.Translate.ItemSaver.prototype = {
 		} else {
 			myNote.setNote(note);
 		}
+		if (this._collections) {
+			myNote.setCollections(this._collections);
+		}
 		yield myNote.save();
 		return myNote;
 	}),

chrome/content/zotero/zoteroPane.js

@@ -3115,11 +3115,10 @@ var ZoteroPane = new function()
 		if (parentKey) {
 			item.parentKey = parentKey;
 		}
-		var itemID = yield item.saveTx();
+		else if (this.collectionsView.selectedTreeRow.isCollection()) {
-		
+			item.addToCollection(this.collectionsView.selectedTreeRow.ref.id);
-		if (!parentKey && this.itemsView && this.collectionsView.selectedTreeRow.isCollection()) {
-			yield this.collectionsView.selectedTreeRow.ref.addItem(itemID);
 		}
+		var itemID = yield item.saveTx();
 		
 		yield this.selectItem(itemID);
 		
@@ -3181,13 +3180,14 @@ var ZoteroPane = new function()
 	});
 	
 	
-	this.createItemAndNoteFromSelectedText = function (event) {
+	this.createItemAndNoteFromSelectedText = Zotero.Promise.coroutine(function* (event) {
 		var str = event.currentTarget.ownerDocument.popupNode.ownerDocument.defaultView.getSelection().toString();
 		var uri = event.currentTarget.ownerDocument.popupNode.ownerDocument.location.href;
-		var itemID = ZoteroPane.addItemFromPage();
+		var item = yield ZoteroPane.addItemFromPage();
-		var {libraryID, key} = Zotero.Items.getLibraryAndKeyFromID(itemID);
+		if (item) {
-		ZoteroPane.newNote(false, key, str, uri)
+			return ZoteroPane.newNote(false, item.key, str, uri)
-	};
+		}
+	});
 	
 	
 	
@@ -3316,28 +3316,33 @@ var ZoteroPane = new function()
 	});
 	
 	
-	this.addItemFromPage = function (itemType, saveSnapshot, row) {
+	/**
-		return Zotero.Promise.try(function () {
+	 * @return {Promise<Zotero.Item>|false}
-			if(Zotero.isConnector) {
+	 */
-				// In connector, save page via Zotero Standalone
+	this.addItemFromPage = Zotero.Promise.method(function (itemType, saveSnapshot, row) {
-				var doc = window.content.document;
+		if(Zotero.isConnector) {
-				Zotero.Connector.callMethod("saveSnapshot", {"url":doc.location.toString(),
+			// In connector, save page via Zotero Standalone
-					"cookie":doc.cookie, "html":doc.documentElement.innerHTML,
+			var doc = window.content.document;
-					"skipSnapshot": saveSnapshot === false || (saveSnapshot === true ? false : undefined)},
+			Zotero.Connector.callMethod("saveSnapshot", {"url":doc.location.toString(),
-				function(returnValue, status) {
+				"cookie":doc.cookie, "html":doc.documentElement.innerHTML,
-					_showPageSaveStatus(doc.title);
+				"skipSnapshot": saveSnapshot === false || (saveSnapshot === true ? false : undefined)},
-				});
+			function(returnValue, status) {
-				return;
+				_showPageSaveStatus(doc.title);
-			}
+			});
+			return false;
+		}
 		
-			if ((row || (this.collectionsView && this.collectionsView.selection)) && !this.canEdit(row)) {
+		if (!row && this.collectionsView && this.collectionsView.selection) {
-				this.displayCannotEditLibraryMessage();
+			row = this.collectionsView.selection.currentIndex;
-				return;
+		}
-			}
 		
-			return this.addItemFromDocument(window.content.document, itemType, saveSnapshot, row);
+		if (!this.canEdit(row)) {
-		}.bind(this));
+			this.displayCannotEditLibraryMessage();
-	}
+			return false;
+		}
+		
+		return this.addItemFromDocument(window.content.document, itemType, saveSnapshot, row);
+	});
 	
 	/**
 	 * Shows progress dialog for a webpage/snapshot save request
@@ -3356,6 +3361,7 @@ var ZoteroPane = new function()
 	 * @param	{String|Integer}	[itemType='webpage']	Item type id or name
 	 * @param	{Boolean}			[saveSnapshot]			Force saving or non-saving of a snapshot,
 	 *														regardless of automaticSnapshots pref
+	 * @return {Promise<Zotero.Item>|false}
 	 */
 	this.addItemFromDocument = Zotero.Promise.coroutine(function* (doc, itemType, saveSnapshot, row) {
 		_showPageSaveStatus(doc.title);
@@ -3399,7 +3405,7 @@ var ZoteroPane = new function()
 				
 				if (row && !this.canEdit(row)) {
 					this.displayCannotEditLibraryMessage();
-					return;
+					return false;
 				}
 				
 				if (row !== undefined) {
@@ -3416,7 +3422,7 @@ var ZoteroPane = new function()
 				
 				if (row && !this.canEditFiles(row)) {
 					this.displayCannotEditLibraryFilesMessage();
-					return;
+					return false;
 				}
 				
 				if (collectionTreeRow && collectionTreeRow.isCollection()) {
@@ -3433,7 +3439,7 @@ var ZoteroPane = new function()
 				});
 				
 				yield this.selectItem(item.id);
-				return;
+				return false;
 			}
 		}
 		
@@ -3467,7 +3473,7 @@ var ZoteroPane = new function()
 			}
 		}
 		
-		return item.id;
+		return item;
 	});
 	
 	

test/tests/browserTest.js

@@ -0,0 +1,51 @@
+"use strict";
+
+describe("Zotero_Browser", function () {
+	var win;
+	before(function* () {
+		win = yield loadBrowserWindow();
+	});
+	after(function* () {
+		win.close();
+	});
+	
+	it("should save webpage item to current collection", function* () {
+		var uri = OS.Path.join(getTestDataDirectory().path, "snapshot", "index.html");
+		var deferred = Zotero.Promise.defer();
+		win.addEventListener('pageshow', () => deferred.resolve());
+		win.loadURI(uri);
+		yield deferred.promise;
+		
+		yield loadZoteroPane(win);
+		var collection = yield createDataObject('collection');
+		
+		var promise = waitForItemEvent('add');
+		yield win.Zotero_Browser.scrapeThisPage();
+		var ids = yield promise;
+		var items = Zotero.Items.get(ids);
+		assert.lengthOf(items, 1);
+		assert.equal(Zotero.ItemTypes.getName(items[0].itemTypeID), 'webpage');
+		assert.isTrue(collection.hasItem(items[0].id));
+	})
+	
+	it("should save journalArticle to current collection", function* () {
+		var uri = OS.Path.join(
+			getTestDataDirectory().path, "metadata", "journalArticle-single.html"
+		);
+		var deferred = Zotero.Promise.defer();
+		win.addEventListener('pageshow', () => deferred.resolve());
+		win.loadURI(uri);
+		yield deferred.promise;
+		
+		yield loadZoteroPane(win);
+		var collection = yield createDataObject('collection');
+		
+		var promise = waitForItemEvent('add');
+		yield win.Zotero_Browser.scrapeThisPage();
+		var ids = yield promise;
+		var items = Zotero.Items.get(ids);
+		assert.lengthOf(items, 1);
+		assert.equal(Zotero.ItemTypes.getName(items[0].itemTypeID), 'journalArticle');
+		assert.isTrue(collection.hasItem(items[0].id));
+	})
+})

test/tests/data/metadata/journalArticle-single.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
+<title>Bibliography</title>
+</head>
+<body>
+<div style="line-height: 1.35; padding-left: 2em; text-indent:-2em;" class="csl-bib-body">
+  <div class="csl-entry">Rosenzweig, Roy. 2003. “Scarcity or Abundance? Preserving the Past in a Digital Era.” <i>The American Historical Review</i> 108 (3): 735–62. doi:10.2307/3523084.</div>
+  <span class="Z3988" title="url_ver=Z39.88-2004&amp;ctx_ver=Z39.88-2004&amp;rfr_id=info%3Asid%2Fzotero.org%3A2&amp;rft_id=info%3Adoi%2F10.2307%2F3523084&amp;rft_val_fmt=info%3Aofi%2Ffmt%3Akev%3Amtx%3Ajournal&amp;rft.genre=article&amp;rft.atitle=Scarcity%20or%20Abundance%3F%20Preserving%20the%20Past%20in%20a%20Digital%20Era&amp;rft.jtitle=The%20American%20Historical%20Review&amp;rft.volume=108&amp;rft.issue=3&amp;rft.aufirst=Roy&amp;rft.aulast=Rosenzweig&amp;rft.au=Roy%20Rosenzweig&amp;rft.date=2003-06&amp;rft.pages=735-762&amp;rft.spage=735&amp;rft.epage=762&amp;rft.issn=00028762"></span>
+</div></body>
+</html>

test/tests/zoteroPaneTest.js

@@ -45,6 +45,15 @@ describe("ZoteroPane", function() {
 			assert.lengthOf(selected, 1);
 			assert.equal(selected, noteID);
 		})
+		
+		it("should create a standalone note within a collection and select it", function* () {
+			var collection = yield createDataObject('collection');
+			var noteID = yield zp.newNote(false, false, "Test");
+			assert.equal(zp.collectionsView.getSelectedCollection(), collection);
+			var selected = zp.itemsView.getSelectedItems(true);
+			assert.lengthOf(selected, 1);
+			assert.equal(selected, noteID);
+		})
 	})
 	
 	describe("#itemSelected()", function () {