From f8b41c971c89ffb474df2103c4bf1a49389a5048 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 27 Feb 2018 01:40:07 -0500
Subject: [PATCH] Automatic attachment file renaming changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Automatic renaming is now done for dragging of an external file onto an
item (as added in 7cb95f41) as well as dragging as a linked file,
dragging a non-native (not text or HTML) URL, "Attach Link to File…",
"Attach Stored Copy of File…", and "Retrieve Metadata for PDF". It only
applies if a single file is being added and if the parent item has no
non-HTML attachments. By default, the renaming only applies to PDFs, but
this can be changed with the renameAttachmentFiles.automatic.fileTypes
hidden pref.

A new General pref, "Automatically rename attachment files using parent
metadata", controls whether the renaming happens.

Files saved via web translators are renamed regardless of this pref,
because they would often be gibberish otherwise.

Closes #113
---
 .../preferences/preferences_general.xul       |   2 +
 chrome/content/zotero/xpcom/attachments.js    |  45 +++++-
 chrome/content/zotero/xpcom/data/item.js      |   9 ++
 chrome/content/zotero/xpcom/itemTreeView.js   |  61 ++++++---
 chrome/content/zotero/xpcom/recognizePDF.js   |  60 +++++---
 chrome/content/zotero/zoteroPane.js           | 102 +++++++++-----
 chrome/locale/en-US/zotero/preferences.dtd    |   1 +
 defaults/preferences/zotero.js                |   2 +
 test/tests/itemTreeViewTest.js                | 128 ++++++++++++++++--
 test/tests/recognizePDFTest.js                |  21 +--
 10 files changed, 335 insertions(+), 96 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences_general.xul b/chrome/content/zotero/preferences/preferences_general.xul
index 2ef9b2814..fc0c087e8 100644
--- a/chrome/content/zotero/preferences/preferences_general.xul
+++ b/chrome/content/zotero/preferences/preferences_general.xul
@@ -37,6 +37,7 @@
 			<preference id="pref-reportTranslationFailure" name="extensions.zotero.reportTranslationFailure" type="bool"/>
 			<preference id="pref-automaticSnapshots" name="extensions.zotero.automaticSnapshots" type="bool"/>
 			<preference id="pref-downloadAssociatedFiles" name="extensions.zotero.downloadAssociatedFiles" type="bool"/>
+			<preference id="pref-renameAttachmentFiles" name="extensions.zotero.renameAttachmentFiles.automatic" type="bool"/>
 			<preference id="pref-automaticTags" name="extensions.zotero.automaticTags" type="bool"/>
 			<preference id="pref-trashAutoEmptyDays" name="extensions.zotero.trashAutoEmptyDays" type="int"/>
 			
@@ -122,6 +123,7 @@
 				label="&zotero.preferences.automaticSnapshots;"
 				preference="pref-automaticSnapshots"/>
 			<checkbox label="&zotero.preferences.downloadAssociatedFiles;" preference="pref-downloadAssociatedFiles"/>
+			<checkbox label="&zotero.preferences.renameAttachmentFiles;" preference="pref-renameAttachmentFiles"/>
 			<checkbox label="&zotero.preferences.automaticTags;" preference="pref-automaticTags"/>
 			<hbox align="center">
 				<label value="&zotero.preferences.trashAutoEmptyDaysPre;"/>
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index 68a3e1b5c..c50cfdd0e 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -245,9 +245,18 @@ Zotero.Attachments = new function(){
 	
 	
 	/**
-	 * @param {Object} options - 'libraryID', 'url', 'parentItemID', 'collections', 'title',
-	 *                           'fileBaseName', 'contentType', 'referrer', 'cookieSandbox',
-	 *                           'saveOptions'
+	 * @param {Object} options
+	 * @param {Integer} options.libraryID
+	 * @param {String} options.url
+	 * @param {Integer} [options.parentItemID]
+	 * @param {Integer[]} [options.collections]
+	 * @param {String} [options.title]
+	 * @param {String} [options.fileBaseName]
+	 * @param {Boolean} [options.renameIfAllowedType=false]
+	 * @param {String} [options.contentType]
+	 * @param {String} [options.referrer]
+	 * @param {CookieSandbox} [options.cookieSandbox]
+	 * @param {Object} [options.saveOptions]
 	 * @return {Promise<Zotero.Item>} - A promise for the created attachment item
 	 */
 	this.importFromURL = Zotero.Promise.coroutine(function* (options) {
@@ -257,6 +266,7 @@ Zotero.Attachments = new function(){
 		var collections = options.collections;
 		var title = options.title;
 		var fileBaseName = options.fileBaseName;
+		var renameIfAllowedType = options.renameIfAllowedType;
 		var contentType = options.contentType;
 		var referrer = options.referrer;
 		var cookieSandbox = options.cookieSandbox;
@@ -320,6 +330,11 @@ Zotero.Attachments = new function(){
 		
 		// Save using remote web browser persist
 		var externalHandlerImport = Zotero.Promise.coroutine(function* (contentType) {
+			// Rename attachment
+			if (renameIfAllowedType && !fileBaseName && this.getRenamedFileTypes().includes(contentType)) {
+				let parentItem = Zotero.Items.get(parentItemID);
+				fileBaseName = this.getFileBaseNameFromItem(parentItem);
+			}
 			if (fileBaseName) {
 				let ext = _getExtensionFromURL(url, contentType);
 				var fileName = fileBaseName + (ext != '' ? '.' + ext : '');
@@ -804,6 +819,30 @@ Zotero.Attachments = new function(){
 	}
 	
 	
+	this.getRenamedFileTypes = function () {
+		try {
+			var types = Zotero.Prefs.get('renameAttachmentFiles.automatic.fileTypes');
+			return types ? types.split(',') : [];
+		}
+		catch (e) {
+			return [];
+		}
+	};
+	
+	
+	this.getRenamedFileBaseNameIfAllowedType = async function (parentItem, file) {
+		var types = this.getRenamedFileTypes();
+		var contentType = file.endsWith('.pdf')
+			// Don't bother reading file if there's a .pdf extension
+			? 'application/pdf'
+			: await Zotero.MIME.getMIMETypeFromFile(file);
+		if (!types.includes(contentType)) {
+			return false;
+		}
+		return this.getFileBaseNameFromItem(parentItem);
+	}
+	
+	
 	/**
 	 * Create directory for attachment files within storage directory
 	 *
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 76828d42a..023bd1cdc 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -2120,6 +2120,15 @@ Zotero.Item.prototype.numAttachments = function (includeTrashed) {
 }
 
 
+Zotero.Item.prototype.numNonHTMLFileAttachments = function () {
+	this._requireData('childItems');
+	return this.getAttachments()
+		.map(itemID => Zotero.Items.get(itemID))
+		.filter(item => item.isFileAttachment() && item.attachmentContentType != 'text/html')
+		.length;
+};
+
+
 Zotero.Item.prototype.getFile = function () {
 	Zotero.debug("Zotero.Item.prototype.getFile() is deprecated -- use getFilePath[Async]()", 2);
 	
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 80aff7a6e..0bb702112 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -3231,26 +3231,22 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 		
 		var notifierQueue = new Zotero.Notifier.Queue;
 		try {
+			// If there's a single file being added to a parent, automatic renaming is enabled,
+			// and there are no other non-HTML attachments, we'll rename the file as long as it's
+			// an allowed type. The dragged data could be a URL, so we don't yet know the file type.
+			// This should be kept in sync with ZoteroPane.addAttachmentFromDialog().
+			let renameIfAllowedType = false;
 			let parentItem;
-			let numExistingFileAttachments;
-			if (parentItemID) {
+			if (parentItemID && data.length == 1 && Zotero.Prefs.get('renameAttachmentFiles.automatic')) {
 				parentItem = Zotero.Items.get(parentItemID);
-				numExistingFileAttachments = parentItem.getAttachments()
-					.map(itemID => Zotero.Items.get(itemID))
-					.filter(item => item.isFileAttachment())
-					.length;
+				if (!parentItem.numNonHTMLFileAttachments()) {
+					renameIfAllowedType = true;
+				}
 			}
 			
 			for (var i=0; i<data.length; i++) {
 				var file = data[i];
 				
-				let fileBaseName;
-				// If only one item is being dragged and it's the only attachment, run
-				// "Rename File from Parent Metadata" automatically
-				if (data.length == 1 && parentItem && !numExistingFileAttachments) {
-					fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
-				}
-				
 				if (dataType == 'text/x-moz-url') {
 					var url = data[i];
 					if (url.indexOf('file:///') == 0) {
@@ -3281,7 +3277,7 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 							yield Zotero.Attachments.importFromURL({
 								libraryID: targetLibraryID,
 								url,
-								fileBaseName,
+								renameIfAllowedType,
 								parentItemID,
 								saveOptions: {
 									notifierQueue
@@ -3298,7 +3294,36 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 					// Otherwise file, so fall through
 				}
 				
+				file = file.path;
+				
+				// Rename file if it's an allowed type
+				let fileBaseName = false;
+				if (renameIfAllowedType) {
+					 fileBaseName = yield Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
+						parentItem, file
+					);
+				}
+				
 				if (dropEffect == 'link') {
+					// Rename linked file, with unique suffix if necessary
+					try {
+						if (fileBaseName) {
+							let ext = Zotero.File.getExtension(file);
+							let newName = yield Zotero.File.rename(
+								file,
+								fileBaseName + (ext ? '.' + ext : ''),
+								{
+									unique: true
+								}
+							);
+							// Update path in case the name was changed to be unique
+							file = OS.Path.join(OS.Path.dirname(file), newName);
+						}
+					}
+					catch (e) {
+						Zotero.logError(e);
+					}
+					
 					yield Zotero.Attachments.linkFromFile({
 						file,
 						parentItemID,
@@ -3309,9 +3334,9 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 					});
 				}
 				else {
-					if (file.leafName.endsWith(".lnk")) {
+					if (file.endsWith(".lnk")) {
 						let win = Services.wm.getMostRecentWindow("navigator:browser");
-						win.ZoteroPane.displayCannotAddShortcutMessage(file.path);
+						win.ZoteroPane.displayCannotAddShortcutMessage(file);
 						continue;
 					}
 					
@@ -3328,10 +3353,10 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 					// If moving, delete original file
 					if (dragData.dropEffect == 'move') {
 						try {
-							file.remove(false);
+							yield OS.File.remove(file);
 						}
 						catch (e) {
-							Components.utils.reportError("Error deleting original file " + file.path + " after drag");
+							Zotero.logError("Error deleting original file " + file + " after drag");
 						}
 					}
 				}
diff --git a/chrome/content/zotero/xpcom/recognizePDF.js b/chrome/content/zotero/xpcom/recognizePDF.js
index 3e072751b..0bd3d8593 100644
--- a/chrome/content/zotero/xpcom/recognizePDF.js
+++ b/chrome/content/zotero/xpcom/recognizePDF.js
@@ -241,30 +241,48 @@ Zotero.RecognizePDF = new function () {
 	 * @return {Promise}
 	 */
 	async function _processItem(itemID) {
-		let item = await Zotero.Items.getAsync(itemID);
+		let attachment = await Zotero.Items.getAsync(itemID);
 		
-		if (!item || item.parentItemID) throw new Zotero.Exception.Alert('recognizePDF.fileNotFound');
-		
-		let newItem = await _recognize(item);
-		
-		if (newItem) {
-			// put new item in same collections as the old one
-			let itemCollections = item.getCollections();
-			await Zotero.DB.executeTransaction(async function () {
-				for (let itemCollection of itemCollections) {
-					let collection = Zotero.Collections.get(itemCollection);
-					await collection.addItem(newItem.id);
-				}
-				
-				// put old item as a child of the new item
-				item.parentID = newItem.id;
-				await item.save();
-			});
-			
-			return newItem
+		if (!attachment || attachment.parentItemID) {
+			throw new Zotero.Exception.Alert('recognizePDF.error');
 		}
 		
-		return null;
+		let parentItem = await _recognize(attachment);
+		if (!parentItem) {
+			return null;
+		}
+		
+		// Put new item in same collections as the old one
+		let collections = attachment.getCollections();
+		await Zotero.DB.executeTransaction(async function () {
+			if (collections.length) {
+				for (let collection of collections) {
+					parentItem.addToCollection(collection.id);
+				}
+				await parentItem.save();
+			}
+			
+			// Put old item as a child of the new item
+			attachment.parentID = parentItem.id;
+			await attachment.save();
+		});
+		
+		// Rename attachment file to match new metadata
+		if (Zotero.Prefs.get('renameAttachmentFiles')) {
+			let path = attachment.getFilePath();
+			let ext = Zotero.File.getExtension(path);
+			let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
+			let newName = fileBaseName + (ext ? '.' + ext : '');
+			let result = await attachment.renameAttachmentFile(newName, false, true);
+			if (result !== true) {
+				throw new Error("Error renaming " + path);
+			}
+			// Rename attachment title
+			attachment.setField('title', newName);
+			await attachment.saveTx();
+		}
+		
+		return parentItem;
 	}
 	
 	/**
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index 6191b97ff..7eeed426e 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -3678,41 +3678,81 @@ var ZoteroPane = new function()
 		var fp = Components.classes["@mozilla.org/filepicker;1"]
         					.createInstance(nsIFilePicker);
 		fp.init(window, Zotero.getString('pane.item.attachments.select'), nsIFilePicker.modeOpenMultiple);
-		fp.appendFilters(Components.interfaces.nsIFilePicker.filterAll);
+		fp.appendFilters(nsIFilePicker.filterAll);
 		
-		if(fp.show() == nsIFilePicker.returnOK)
-		{
-			if (!parentItemID) {
-				var collection = this.getSelectedCollection(true);
+		if (fp.show() != nsIFilePicker.returnOK) {
+			return;
+		}
+		
+		var enumerator = fp.files;
+		var files = [];
+		while (enumerator.hasMoreElements()) {
+			let file = enumerator.getNext();
+			file.QueryInterface(Components.interfaces.nsIFile);
+			files.push(file.path);
+		}
+		
+		var collection;
+		var fileBaseName;
+		if (parentItemID) {
+			// If only one item is being added, automatic renaming is enabled, and the parent item
+			// doesn't have any other non-HTML file attachments, rename the file.
+			// This should be kept in sync with itemTreeView::drop().
+			if (files.length == 1 && Zotero.Prefs.get('renameAttachmentFiles.automatic')) {
+				let parentItem = Zotero.Items.get(parentItemID);
+				if (!parentItem.numNonHTMLFileAttachments()) {
+					fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
+						parentItem, files[0]
+					);
+				}
 			}
-			
-			var files = fp.files;
-			while (files.hasMoreElements()){
-				var file = files.getNext();
-				file.QueryInterface(Components.interfaces.nsILocalFile);
-				var attachmentID;
-				if (link) {
-					yield Zotero.Attachments.linkFromFile({
-						file: file,
-						parentItemID: parentItemID,
-						collections: collection ? [collection] : undefined
-					});
-				}
-				else {
-					if (file.leafName.endsWith(".lnk")) {
-						let wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
-						.getService(Components.interfaces.nsIWindowMediator);
-						let win = wm.getMostRecentWindow("navigator:browser");
-						win.ZoteroPane.displayCannotAddShortcutMessage(file.path);
-						continue;
+		}
+		// If not adding to an item, add to the current collection
+		else {
+			collection = this.getSelectedCollection(true);
+		}
+		
+		for (let file of files) {
+			if (link) {
+				// Rename linked file, with unique suffix if necessary
+				try {
+					if (fileBaseName) {
+						let ext = Zotero.File.getExtension(file);
+						let newName = yield Zotero.File.rename(
+							file,
+							fileBaseName + (ext ? '.' + ext : ''),
+							{
+								unique: true
+							}
+						);
+						// Update path in case the name was changed to be unique
+						file = OS.Path.join(OS.Path.dirname(file), newName);
 					}
-					yield Zotero.Attachments.importFromFile({
-						file: file,
-						libraryID: libraryID,
-						parentItemID: parentItemID,
-						collections: collection ? [collection] : undefined
-					});
 				}
+				catch (e) {
+					Zotero.logError(e);
+				}
+				
+				let item = yield Zotero.Attachments.linkFromFile({
+					file,
+					parentItemID,
+					collections: collection ? [collection] : undefined
+				});
+			}
+			else {
+				if (file.endsWith(".lnk")) {
+					let win = Services.wm.getMostRecentWindow("navigator:browser");
+					win.ZoteroPane.displayCannotAddShortcutMessage(file);
+					continue;
+				}
+				
+				yield Zotero.Attachments.importFromFile({
+					file,
+					libraryID,
+					fileBaseName,
+					parentItemID,
+					collections: collection ? [collection] : undefined
+				});
 			}
 		}
 	});
diff --git a/chrome/locale/en-US/zotero/preferences.dtd b/chrome/locale/en-US/zotero/preferences.dtd
index b9ff71cc8..9bb38feea 100644
--- a/chrome/locale/en-US/zotero/preferences.dtd
+++ b/chrome/locale/en-US/zotero/preferences.dtd
@@ -27,6 +27,7 @@
 <!ENTITY zotero.preferences.parseRISRefer				"Use Zotero for downloaded BibTeX/RIS/Refer files">
 <!ENTITY zotero.preferences.automaticSnapshots			"Automatically take snapshots when creating items from web pages">
 <!ENTITY zotero.preferences.downloadAssociatedFiles		"Automatically attach associated PDFs and other files when saving items">
+<!ENTITY zotero.preferences.renameAttachmentFiles     "Automatically rename attachment files using parent metadata">
 <!ENTITY zotero.preferences.automaticTags				"Automatically tag items with keywords and subject headings">
 <!ENTITY zotero.preferences.trashAutoEmptyDaysPre		"Automatically remove items in the trash deleted more than">
 <!ENTITY zotero.preferences.trashAutoEmptyDaysPost		"days ago">
diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js
index 4946ddeef..38760ba7f 100644
--- a/defaults/preferences/zotero.js
+++ b/defaults/preferences/zotero.js
@@ -35,6 +35,8 @@ pref("extensions.zotero.automaticTags",true);
 pref("extensions.zotero.fontSize", "1.0");
 pref("extensions.zotero.layout", "standard");
 pref("extensions.zotero.recursiveCollections", false);
+pref("extensions.zotero.renameAttachmentFiles.automatic", true);
+pref("extensions.zotero.renameAttachmentFiles.automatic.fileTypes", "application/pdf");
 pref("extensions.zotero.attachmentRenameFormatString", '{%c - }{%y - }{%t{50}}');
 pref("extensions.zotero.capitalizeTitles", false);
 pref("extensions.zotero.launchNonNativeFiles", false);
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index 3f474892d..ca485efb4 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -695,6 +695,12 @@ describe("Zotero.ItemTreeView", function() {
 			file.append(pdfFilename);
 			pdfPath = file.path;
 			httpd.registerFile("/" + pdfFilename, file);
+			
+			Zotero.Prefs.clear('renameAttachmentFiles.automatic');
+		});
+		
+		afterEach(() => {
+			Zotero.Prefs.clear('renameAttachmentFiles.automatic');
 		});
 		
 		after(function* () {
@@ -781,7 +787,7 @@ describe("Zotero.ItemTreeView", function() {
 			assert.isFalse(view.isContainerEmpty(view.getRowIndexByID(item1.id)));
 		});
 		
-		it("should create a top-level attachment when a file is dragged", function* () {
+		it("should create a stored top-level attachment when a file is dragged", function* () {
 			var file = getTestDataDirectory();
 			file.append('test.png');
 			
@@ -812,7 +818,7 @@ describe("Zotero.ItemTreeView", function() {
 			);
 		});
 		
-		it("should create a top-level attachment when a URL is dragged", function* () {
+		it("should create a stored top-level attachment when a URL is dragged", function* () {
 			var promise = itemsView.waitForSelect();
 			
 			itemsView.drop(0, -1, {
@@ -840,7 +846,7 @@ describe("Zotero.ItemTreeView", function() {
 			);
 		});
 		
-		it("should create a child attachment when a URL is dragged", function* () {
+		it("should create a stored child attachment when a URL is dragged", function* () {
 			var view = zp.itemsView;
 			var parentItem = yield createDataObject('item');
 			var parentRow = view.getRowIndexByID(parentItem.id);
@@ -873,7 +879,7 @@ describe("Zotero.ItemTreeView", function() {
 			);
 		});
 		
-		it("should rename a child attachment using parent metadata if no existing file attachments", async function () {
+		it("should rename a stored child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
 			var view = zp.itemsView;
 			var parentTitle = Zotero.Utilities.randomString();
 			var parentItem = await createDataObject('item', { title: parentTitle });
@@ -885,7 +891,7 @@ describe("Zotero.ItemTreeView", function() {
 			var parentRow = view.getRowIndexByID(parentItem.id);
 			
 			var file = getTestDataDirectory();
-			file.append('test.png');
+			file.append('empty.pdf');
 			
 			var promise = waitForItemEvent('add');
 			
@@ -910,11 +916,101 @@ describe("Zotero.ItemTreeView", function() {
 			assert.equal(item.parentItemID, parentItem.id);
 			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, parentTitle + '.png');
-			assert.equal(OS.Path.basename(path), parentTitle + '.png');
+			assert.equal(title, parentTitle + '.pdf');
+			assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
 		});
 		
-		it("shouldn't rename a child attachment using parent metadata if existing file attachments", async function () {
+		it("should rename a linked child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
+			var view = zp.itemsView;
+			var parentTitle = Zotero.Utilities.randomString();
+			var parentItem = await createDataObject('item', { title: parentTitle });
+			await Zotero.Attachments.linkFromURL({
+				url: 'https://example.com',
+				title: 'Example',
+				parentItemID: parentItem.id
+			});
+			var parentRow = view.getRowIndexByID(parentItem.id);
+			
+			var file = OS.Path.join(await getTempDirectory(), 'empty.pdf');
+			await OS.File.copy(
+				OS.Path.join(getTestDataDirectory().path, 'empty.pdf'),
+				file
+			);
+			file = Zotero.File.pathToFile(file);
+			
+			var promise = waitForItemEvent('add');
+			
+			itemsView.drop(parentRow, 0, {
+				dropEffect: 'link',
+				effectAllowed: 'link',
+				types: {
+					contains: function (type) {
+						return type == 'application/x-moz-file';
+					}
+				},
+				mozItemCount: 1,
+				mozGetDataAt: function (type, i) {
+					if (type == 'application/x-moz-file' && i == 0) {
+						return file;
+					}
+				}
+			})
+			
+			var itemIDs = await promise;
+			var item = Zotero.Items.get(itemIDs[0]);
+			assert.equal(item.parentItemID, parentItem.id);
+			var title = item.getField('title');
+			var path = await item.getFilePathAsync();
+			assert.equal(title, parentTitle + '.pdf');
+			assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
+		});
+		
+		it("shouldn't rename a stored child attachment using parent metadata if pref disabled", async function () {
+			Zotero.Prefs.set('renameAttachmentFiles.automatic', false);
+			
+			var view = zp.itemsView;
+			var parentTitle = Zotero.Utilities.randomString();
+			var parentItem = await createDataObject('item', { title: parentTitle });
+			await Zotero.Attachments.linkFromURL({
+				url: 'https://example.com',
+				title: 'Example',
+				parentItemID: parentItem.id
+			});
+			var parentRow = view.getRowIndexByID(parentItem.id);
+			
+			var originalFileName = 'empty.pdf';
+			var file = getTestDataDirectory();
+			file.append(originalFileName);
+			
+			var promise = waitForItemEvent('add');
+			
+			itemsView.drop(parentRow, 0, {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				types: {
+					contains: function (type) {
+						return type == 'application/x-moz-file';
+					}
+				},
+				mozItemCount: 1,
+				mozGetDataAt: function (type, i) {
+					if (type == 'application/x-moz-file' && i == 0) {
+						return file;
+					}
+				}
+			})
+			
+			var itemIDs = await promise;
+			var item = Zotero.Items.get(itemIDs[0]);
+			assert.equal(item.parentItemID, parentItem.id);
+			var title = item.getField('title');
+			var path = await item.getFilePathAsync();
+			// Should match original filename, not parent title
+			assert.equal(title, originalFileName);
+			assert.equal(OS.Path.basename(path), originalFileName);
+		});
+		
+		it("shouldn't rename a stored child attachment using parent metadata if existing file attachments", async function () {
 			var view = zp.itemsView;
 			var parentTitle = Zotero.Utilities.randomString();
 			var parentItem = await createDataObject('item', { title: parentTitle });
@@ -924,8 +1020,9 @@ describe("Zotero.ItemTreeView", function() {
 			});
 			var parentRow = view.getRowIndexByID(parentItem.id);
 			
+			var originalFileName = 'empty.pdf';
 			var file = getTestDataDirectory();
-			file.append('test.png');
+			file.append(originalFileName);
 			
 			var promise = waitForItemEvent('add');
 			
@@ -950,18 +1047,19 @@ describe("Zotero.ItemTreeView", function() {
 			assert.equal(item.parentItemID, parentItem.id);
 			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, 'test.png');
-			assert.equal(OS.Path.basename(path), 'test.png');
+			assert.equal(title, originalFileName);
+			assert.equal(OS.Path.basename(path), originalFileName);
 		});
 		
-		it("shouldn't rename a child attachment using parent metadata if drag includes multiple files", async function () {
+		it("shouldn't rename a stored child attachment using parent metadata if drag includes multiple files", async function () {
 			var view = zp.itemsView;
 			var parentTitle = Zotero.Utilities.randomString();
 			var parentItem = await createDataObject('item', { title: parentTitle });
 			var parentRow = view.getRowIndexByID(parentItem.id);
 			
+			var originalFileName = 'empty.pdf';
 			var file = getTestDataDirectory();
-			file.append('test.png');
+			file.append(originalFileName);
 			
 			var promise = waitForItemEvent('add');
 			
@@ -986,8 +1084,8 @@ describe("Zotero.ItemTreeView", function() {
 			assert.equal(item.parentItemID, parentItem.id);
 			var title = item.getField('title');
 			var path = await item.getFilePathAsync();
-			assert.equal(title, 'test.png');
-			assert.equal(OS.Path.basename(path), 'test.png');
+			assert.equal(title, originalFileName);
+			assert.equal(OS.Path.basename(path), originalFileName);
 		});
 	});
 })
diff --git a/test/tests/recognizePDFTest.js b/test/tests/recognizePDFTest.js
index 092649c24..5bc9d4ae6 100644
--- a/test/tests/recognizePDFTest.js
+++ b/test/tests/recognizePDFTest.js
@@ -27,31 +27,36 @@ describe("PDF Recognition", function() {
 		}
 	});
 
-	it("should recognize a PDF", function* () {
+	it("should recognize a PDF", async function () {
 		this.timeout(30000);
 		// Import the PDF
 		var testdir = getTestDataDirectory();
 		testdir.append("recognizePDF_test_GS.pdf");
-		var item = yield Zotero.Attachments.importFromFile({
+		var attachment = await Zotero.Attachments.importFromFile({
 			file: testdir
 		});
 		
 		// Recognize the PDF
 		win.ZoteroPane.recognizeSelected();
 		
-		var addedIDs = yield waitForItemEvent("add");
-		var modifiedIDs = yield waitForItemEvent("modify");
+		var addedIDs = await waitForItemEvent("add");
+		var modifiedIDs = await waitForItemEvent("modify");
 		assert.lengthOf(addedIDs, 1);
 		var item = Zotero.Items.get(addedIDs[0]);
 		assert.equal(item.getField("title"), "Scaling study of an improved fermion action on quenched lattices");
 		assert.lengthOf(modifiedIDs, 2);
 		
-		yield Zotero.Promise.delay(0);
-		
+		// Wait for status to show as complete
 		var progressWindow = getWindows("chrome://zotero/content/recognizePDFDialog.xul")[0];
+		var completeStr = Zotero.getString("recognizePDF.complete.label");
+		while (progressWindow.document.getElementById("label").value != completeStr) {
+			await Zotero.Promise.delay(20);
+		}
+		
+		// The file should have been renamed
 		assert.equal(
-			progressWindow.document.getElementById("label").value,
-			Zotero.getString("recognizePDF.complete.label")
+			attachment.attachmentFilename,
+			Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
 		);
 	});
 });
\ No newline at end of file