From d0f7fd6df780b93e8f41448e76f3fd02c5c70ad2 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 26 Feb 2019 22:23:43 -0500
Subject: [PATCH] Add separate "Rename linked files" pref for auto-renaming

Defaulting to false

People who've been relying on auto-renaming of linked files for the last
year will have to turn this back on.
---
 .../zotero/preferences/preferences_general.js |  7 +++
 .../preferences/preferences_general.xul       |  9 +++-
 chrome/content/zotero/xpcom/attachments.js    | 11 ++++
 chrome/content/zotero/xpcom/itemTreeView.js   |  4 +-
 chrome/content/zotero/xpcom/recognizePDF.js   |  2 +-
 chrome/content/zotero/zoteroPane.js           |  2 +-
 chrome/locale/en-US/zotero/preferences.dtd    |  1 +
 chrome/skin/default/zotero/preferences.css    |  8 +++
 defaults/preferences/zotero.js                |  1 +
 test/tests/itemTreeViewTest.js                | 50 ++++++++++++++++++-
 10 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences_general.js b/chrome/content/zotero/preferences/preferences_general.js
index a0f8706ba0..883cf8e7f1 100644
--- a/chrome/content/zotero/preferences/preferences_general.js
+++ b/chrome/content/zotero/preferences/preferences_general.js
@@ -40,9 +40,16 @@ Zotero_Preferences.General = {
 		
 		document.getElementById('noteFontSize').value = Zotero.Prefs.get('note.fontSize');
 		
+		this.updateAutoRenameFilesUI();
 		this._updateFileHandlerUI();
 	},
 	
+	updateAutoRenameFilesUI: function () {
+		setTimeout(() => {
+			document.getElementById('rename-linked-files').disabled = !Zotero.Prefs.get('autoRenameFiles');
+		});
+	},
+	
 	//
 	// File handlers
 	//
diff --git a/chrome/content/zotero/preferences/preferences_general.xul b/chrome/content/zotero/preferences/preferences_general.xul
index 1233e63907..aa021b424f 100644
--- a/chrome/content/zotero/preferences/preferences_general.xul
+++ b/chrome/content/zotero/preferences/preferences_general.xul
@@ -38,6 +38,7 @@
 			<preference id="pref-downloadAssociatedFiles" name="extensions.zotero.downloadAssociatedFiles" type="bool"/>
 			<preference id="pref-autoRecognizeFiles" name="extensions.zotero.autoRecognizeFiles" type="bool"/>
 			<preference id="pref-autoRenameFiles" name="extensions.zotero.autoRenameFiles" type="bool"/>
+			<preference id="pref-autoRenameFilesLinked" name="extensions.zotero.autoRenameFiles.linked" type="bool"/>
 			
 			<preference id="pref-automaticTags" name="extensions.zotero.automaticTags" type="bool"/>
 			<preference id="pref-trashAutoEmptyDays" name="extensions.zotero.trashAutoEmptyDays" type="int"/>
@@ -118,7 +119,13 @@
 				preference="pref-automaticSnapshots"/>
 			<checkbox label="&zotero.preferences.downloadAssociatedFiles;" preference="pref-downloadAssociatedFiles"/>
 			<checkbox label="&zotero.preferences.autoRecognizeFiles;" preference="pref-autoRecognizeFiles"/>
-			<checkbox label="&zotero.preferences.autoRenameFiles;" preference="pref-autoRenameFiles"/>
+			<checkbox label="&zotero.preferences.autoRenameFiles;"
+					preference="pref-autoRenameFiles"
+					oncommand="Zotero_Preferences.General.updateAutoRenameFilesUI()"/>
+			<checkbox id="rename-linked-files" class="indented-pref"
+					label="&zotero.preferences.autoRenameFiles.renameLinked;"
+					preference="pref-autoRenameFilesLinked"
+					oncommand="Zotero_Preferences.General.updateAutoRenameFilesUI()"/>
 			
 			<hbox align="center">
 				<label value="&zotero.preferences.fileHandler.openPDFsUsing;" control="file-handler-pdf"/>
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index bf7f42e3ee..a293c602f2 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -1913,6 +1913,17 @@ Zotero.Attachments = new function(){
 	}
 	
 	
+	this.shouldAutoRenameFile = function (isLink) {
+		if (!Zotero.Prefs.get('autoRenameFiles')) {
+			return false;
+		}
+		if (isLink) {
+			return Zotero.Prefs.get('autoRenameFiles.linked');
+		}
+		return true;
+	}
+	
+	
 	this.getRenamedFileTypes = function () {
 		try {
 			var types = Zotero.Prefs.get('autoRenameFiles.fileTypes');
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index da490957ac..4fb39fe712 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -3282,7 +3282,9 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 			// This should be kept in sync with ZoteroPane.addAttachmentFromDialog().
 			let renameIfAllowedType = false;
 			let parentItem;
-			if (parentItemID && data.length == 1 && Zotero.Prefs.get('autoRenameFiles')) {
+			if (parentItemID
+					&& data.length == 1
+					&& Zotero.Attachments.shouldAutoRenameFile(dropEffect == 'link')) {
 				parentItem = Zotero.Items.get(parentItemID);
 				if (!parentItem.numNonHTMLFileAttachments()) {
 					renameIfAllowedType = true;
diff --git a/chrome/content/zotero/xpcom/recognizePDF.js b/chrome/content/zotero/xpcom/recognizePDF.js
index d9de4a9f47..fff1f8ff16 100644
--- a/chrome/content/zotero/xpcom/recognizePDF.js
+++ b/chrome/content/zotero/xpcom/recognizePDF.js
@@ -287,7 +287,7 @@ Zotero.RecognizePDF = new function () {
 		var originalFilename = OS.Path.basename(path);
 		
 		// Rename attachment file to match new metadata
-		if (Zotero.Prefs.get('autoRenameFiles')) {
+		if (Zotero.Attachments.shouldAutoRenameFile(attachment.linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE)) {
 			let ext = Zotero.File.getExtension(path);
 			let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
 			let newName = fileBaseName + (ext ? '.' + ext : '');
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index 6b884b1397..a83e98a0f9 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -3577,7 +3577,7 @@ var ZoteroPane = new function()
 			// 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('autoRenameFiles')) {
+			if (files.length == 1 && Zotero.Attachments.shouldAutoRenameFile(link)) {
 				let parentItem = Zotero.Items.get(parentItemID);
 				if (!parentItem.numNonHTMLFileAttachments()) {
 					fileBaseName = yield Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
diff --git a/chrome/locale/en-US/zotero/preferences.dtd b/chrome/locale/en-US/zotero/preferences.dtd
index f4edf66451..cb031c476b 100644
--- a/chrome/locale/en-US/zotero/preferences.dtd
+++ b/chrome/locale/en-US/zotero/preferences.dtd
@@ -24,6 +24,7 @@
 <!ENTITY zotero.preferences.downloadAssociatedFiles		"Automatically attach associated PDFs and other files when saving items">
 <!ENTITY zotero.preferences.autoRecognizeFiles        "Automatically retrieve metadata for PDFs">
 <!ENTITY zotero.preferences.autoRenameFiles     "Automatically rename attachment files using parent metadata">
+<!ENTITY zotero.preferences.autoRenameFiles.renameLinked  "Rename linked files">
 <!ENTITY zotero.preferences.fileHandler.openPDFsUsing "Open PDFs using">
 <!ENTITY zotero.preferences.fileHandler.systemDefault "System Default">
 
diff --git a/chrome/skin/default/zotero/preferences.css b/chrome/skin/default/zotero/preferences.css
index 37ba1ad256..e1f5364e95 100644
--- a/chrome/skin/default/zotero/preferences.css
+++ b/chrome/skin/default/zotero/preferences.css
@@ -19,6 +19,10 @@ radio[pane]
 	-moz-box-pack: end;
 }
 
+checkbox[disabled="true"] {
+	color: gray !important;
+}
+
 #export-citePaperJournalArticleURL
 {
 	font-size: .85em;
@@ -83,6 +87,10 @@ grid row hbox:first-child
 	margin-left: .75em;
 }
 
+.indented-pref {
+	margin-left: 2em;
+}
+
 .fileHandler-menu .menulist-icon {
 	height: 16px;
 }
diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js
index 00f2671ad5..72e7e4c476 100644
--- a/defaults/preferences/zotero.js
+++ b/defaults/preferences/zotero.js
@@ -35,6 +35,7 @@ pref("extensions.zotero.layout", "standard");
 pref("extensions.zotero.recursiveCollections", false);
 pref("extensions.zotero.autoRecognizeFiles", true);
 pref("extensions.zotero.autoRenameFiles", true);
+pref("extensions.zotero.autoRenameFiles.linked", false);
 pref("extensions.zotero.autoRenameFiles.fileTypes", "application/pdf");
 pref("extensions.zotero.attachmentRenameFormatString", "{%c - }{%y - }{%t{50}}");
 pref("extensions.zotero.capitalizeTitles", false);
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index de9256b719..f4a2776295 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -755,6 +755,7 @@ describe("Zotero.ItemTreeView", function() {
 			// Don't run recognize on every file
 			Zotero.Prefs.set('autoRecognizeFiles', false);
 			Zotero.Prefs.clear('autoRenameFiles');
+			Zotero.Prefs.clear('autoRenameFiles.linked');
 		});
 		
 		after(function* () {
@@ -764,6 +765,7 @@ describe("Zotero.ItemTreeView", function() {
 			
 			Zotero.Prefs.clear('autoRecognizeFiles');
 			Zotero.Prefs.clear('autoRenameFiles');
+			Zotero.Prefs.clear('autoRenameFiles.linked');
 		});
 		
 		it("should move a child item from one item to another", function* () {
@@ -1091,6 +1093,8 @@ describe("Zotero.ItemTreeView", function() {
 		});
 		
 		it("should rename a linked child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
+			Zotero.Prefs.set('autoRenameFiles.linked', true);
+			
 			var view = zp.itemsView;
 			var parentTitle = Zotero.Utilities.randomString();
 			var parentItem = await createDataObject('item', { title: parentTitle });
@@ -1135,8 +1139,8 @@ describe("Zotero.ItemTreeView", function() {
 			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('autoRenameFiles', false);
+		it("shouldn't rename a linked child attachment using parent metadata if no existing file attachments and pref disabled", async function () {
+			Zotero.Prefs.set('autoRenameFiles.linked', false);
 			
 			var view = zp.itemsView;
 			var parentTitle = Zotero.Utilities.randomString();
@@ -1148,6 +1152,48 @@ describe("Zotero.ItemTreeView", function() {
 			});
 			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, 'empty.pdf');
+			assert.equal(OS.Path.basename(path), 'empty.pdf');
+		});
+		
+		it("shouldn't rename a stored child attachment using parent metadata if pref disabled", async function () {
+			Zotero.Prefs.set('autoRenameFiles', false);
+			
+			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(originalFileName);