www

commit 8e276b30d4557212cedd176c52816ce694af690a
parent 64fb9efa760d22c83f2875b19a57c6381a3e81a0
Author: Dan Stillman <dstillman@gmail.com>
Date:   Sat, 23 Nov 2013 22:28:06 -0800

Merge pull request #429 from aurimasv/rename-attachment

When renaming attachment files, keep file extension if not provided
Diffstat:
M
chrome/content/zotero/bindings/attachmentbox.xml
 | 
26
+++++++++++++++++---------
M
chrome/content/zotero/xpcom/data/item.js
 | 
14
++++++++------

2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/chrome/content/zotero/bindings/attachmentbox.xml b/chrome/content/zotero/bindings/attachmentbox.xml
@@ -388,22 +388,30 @@
 						
 						// Rename associated file
 						if (checkState.value) {
-							var renamed = item.renameAttachmentFile(newTitle.value);
+							var newFilename = newTitle.value.trim();
+							if (newFilename.search(/\.\w{1,10}$/) == -1) {
+								// User did not specify extension. Use current
+								var oldExt = item.getFilename().match(/\.\w{1,10}$/);
+								if (oldExt) newFilename += oldExt[0];
+							}
+							var renamed = item.renameAttachmentFile(newFilename);
 							if (renamed == -1) {
 								var confirmed = nsIPS.confirm(
 									window,
 									'',
-									newTitle.value + ' exists. Overwrite existing file?'
+									newFilename + ' exists. Overwrite existing file?'
 								);
-								if (confirmed) {
-									item.renameAttachmentFile(newTitle.value, true);
-									break;
+								if (!confirmed) {
+									// If they said not to overwrite existing file,
+									// start again
+									continue;
 								}
-								// If they said not to overwrite existing file,
-								// start again
-								continue;
+								
+								// Force overwrite, but make sure we check that this doesn't fail
+								renamed = item.renameAttachmentFile(newFilename, true);
 							}
-							else if (renamed == -2) {
+							
+							if (renamed == -2) {
 								nsIPS.alert(
 									window,
 									Zotero.getString('general.error'),
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -3009,6 +3009,7 @@ Zotero.Item.prototype.renameAttachmentFile = function(newName, overwrite) {
 		return false;
 	}
 	
+	var origModDate = file.lastModifiedTime;
 	try {
 		newName = Zotero.File.getValidFileName(newName);
 		
@@ -3027,18 +3028,17 @@ Zotero.Item.prototype.renameAttachmentFile = function(newName, overwrite) {
 		// files, since dest.exists() will just show true on a case-insensitive
 		// filesystem anyway.
 		if (file.leafName.toLowerCase() != dest.leafName.toLowerCase()) {
-			if (overwrite) {
-				dest.remove(false);
-			}
-			else if (dest.exists()) {
+			if (!overwrite && dest.exists()) {
 				return -1;
 			}
 		}
 		
-		file.moveTo(null, newName);
 		// Update mod time and clear hash so the file syncs
 		// TODO: use an integer counter instead of mod time for change detection
-		dest.lastModifiedTime = new Date();
+		// Update mod time first, because it may fail for read-only files on Windows
+		file.lastModifiedTime = new Date();
+		file.moveTo(null, newName);
+		
 		this.relinkAttachmentFile(dest);
 		
 		Zotero.DB.beginTransaction();
@@ -3051,6 +3051,8 @@ Zotero.Item.prototype.renameAttachmentFile = function(newName, overwrite) {
 		return true;
 	}
 	catch (e) {
+		// Restore original modification date in case we managed to change it
+		try { file.lastModifiedTime = origModDate } catch (e) {}
 		Zotero.debug(e);
 		Components.utils.reportError(e);
 		return -2;