From 27a2a9c1f7e9208ba54f225d0b30efc54ca10f4c Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 21 Feb 2013 04:40:45 -0500
Subject: [PATCH] More relative path changes

- If attachment exists at same relative path in new base directory,
  leave it alone so that it continues to work.
- If attachment doesn't exist in new base directory, revert it to an
  absolute path.
- If new base directory is an ancestor or descendant of the previous
  base directory, adjust relative paths below the new directory so that
  they keep working.
- More dialog changes
- Select current base directory in file picker when changing directory
- Always use .persistentDescriptor instead of initWithPath(), though it
  probably doesn't matter, and wrap in try/catch in case the old setting
  is broken in some way.
- New function Zotero.File.directoryContains(dir, file), since
  nsIFile.contains() isn't recursive
- Don't use a private Zotero.Item property from outside to force path changes.
---
 .../content/zotero/preferences/preferences.js | 127 +++++++++++++-----
 .../zotero/preferences/preferences.xul        |   2 +-
 chrome/content/zotero/xpcom/data/item.js      | 119 +++++++++++-----
 chrome/content/zotero/xpcom/file.js           |  26 ++++
 chrome/content/zotero/zoteroPane.js           |   1 +
 chrome/locale/en-US/zotero/preferences.dtd    |   2 +-
 chrome/locale/en-US/zotero/zotero.properties  |  22 +--
 7 files changed, 219 insertions(+), 80 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences.js b/chrome/content/zotero/preferences/preferences.js
index 43a018dd7..1b293ed43 100644
--- a/chrome/content/zotero/preferences/preferences.js
+++ b/chrome/content/zotero/preferences/preferences.js
@@ -131,16 +131,39 @@ function init()
 
 
 function chooseBaseAttachmentPath() {
+	// Get existing base directory
+	var oldBasePath = Zotero.Prefs.get('baseAttachmentPath');
+	if (oldBasePath) {
+		var oldBasePathFile = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			oldBasePathFile.persistentDescriptor = oldBasePath;
+		}
+		catch (e) {
+			Zotero.debug(e, 1);
+			Components.utils.reportError(e);
+			oldBasePathFile = null;
+		}
+	}
+	
 	//Prompt user to choose new base path
 	var nsIFilePicker = Components.interfaces.nsIFilePicker;
 	var fp = Components.classes["@mozilla.org/filepicker;1"]
 				.createInstance(nsIFilePicker);
+	if (oldBasePathFile) {
+		fp.displayDirectory = oldBasePathFile;
+	}
 	fp.init(window, Zotero.getString('attachmentBasePath.selectDir'), nsIFilePicker.modeGetFolder);
 	fp.appendFilters(nsIFilePicker.filterAll);
 	if (fp.show() != nsIFilePicker.returnOK) {
 		return false;
 	}
-	var basePathFile = fp.file;
+	var newBasePathFile = fp.file;
+	
+	if (oldBasePathFile && oldBasePathFile.equals(newBasePathFile)) {
+		Zotero.debug("Base directory hasn't changed");
+		return false;
+	}
 	
 	//Find all current attachments with relative attachment paths
 	var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=? AND path LIKE '" + 
@@ -152,28 +175,49 @@ function chooseBaseAttachmentPath() {
 	var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=?";
 	var params=[Zotero.Attachments.LINK_MODE_LINKED_FILE];
 	var allAttachments = Zotero.DB.columnQuery(sql,params);
-	var newRelativeAttachmentIDs = [];
+	var newAttachmentPaths = {};
+	var numNewAttachments = 0;
+	var numOldAttachments = 0;
 	var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
 		.createInstance(Components.interfaces.nsILocalFile);
-	var tempAttachmentID,tempAttachment;
-	for (var index=0; index<allAttachments.length; index++) {
-		tempAttachmentID=allAttachments[index];
-		
-		//Don't count current relative attachment paths.  
-		if (oldRelativeAttachmentIDs.indexOf(tempAttachmentID)!=-1) {
-			continue;
-		}
+	for (let i=0; i<allAttachments.length; i++) {
+		let attachmentID = allAttachments[i];
 		
 		try {
-			tempAttachment=Zotero.Items.get(tempAttachmentID);
-			attachmentFile.initWithPath(tempAttachment.attachmentPath);
+			let attachment = Zotero.Items.get(attachmentID);
+			attachmentFile.persistentDescriptor = attachment.attachmentPath;
 		}
 		catch (e) {
 			//Don't deal with bad attachment paths.  Just skip them.
 			continue;
 		}
-		if (basePathFile.contains(attachmentFile,false)) {
-			newRelativeAttachmentIDs.push(allAttachments[index]);
+		
+		// If a file with the same relative path exists within the new base directory,
+		// don't touch the attachment, since it will continue to work
+		let isExistingRelativeAttachment = oldRelativeAttachmentIDs.indexOf(attachmentID) != -1;
+		if (isExistingRelativeAttachment && oldBasePathFile) {
+			let relFile = attachmentFile.clone();
+			let relPath = attachmentFile.getRelativeDescriptor(oldBasePathFile);
+			relFile.setRelativeDescriptor(newBasePathFile, relPath);
+			if (relFile.exists()) {
+				numNewAttachments++;
+				continue;
+			}
+		}
+		
+		// Files within the new base directory need to be updated to use
+		// relative paths (or, if the new base directory is an ancestor or
+		// descendant of the old one, new relative paths)
+		if (Zotero.File.directoryContains(newBasePathFile, attachmentFile)) {
+			newAttachmentPaths[attachmentID] = isExistingRelativeAttachment
+				? attachmentFile.persistentDescriptor : null;
+			numNewAttachments++;
+		}
+		// Existing relative attachments not within the new base directory
+		// will be converted to absolute paths
+		else if (isExistingRelativeAttachment && oldBasePathFile) {
+			newAttachmentPaths[attachmentID] = attachmentFile.persistentDescriptor;
+			numOldAttachments++;
 		}
 	}
 	
@@ -181,30 +225,43 @@ function chooseBaseAttachmentPath() {
 	var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
 		.getService(Components.interfaces.nsIPromptService);
 	
-	var strPrefix = 'attachmentBasePath.confirmNewPath.';
-	var confirmTitle = Zotero.getString(strPrefix + 'title');
-	var confirmString = Zotero.getString(strPrefix + 'message');
-	switch (newRelativeAttachmentIDs.length) {
+	var chooseStrPrefix = 'attachmentBasePath.chooseNewPath.';
+	var clearStrPrefix = 'attachmentBasePath.clearBasePath.';
+	var title = Zotero.getString(chooseStrPrefix + 'title');
+	var msg1 = Zotero.getString(chooseStrPrefix + 'message') + "\n\n", msg2 = msg3 = "";
+	switch (numNewAttachments) {
 		case 0:
 			break;
 		
 		case 1:
-			confirmString += "\n\n" + Zotero.getString(strPrefix + 'existingAttachments.singular');
+			msg2 += Zotero.getString(chooseStrPrefix + 'existingAttachments.singular') + " ";
 			break;
 		
 		default:
-			confirmString += "\n\n" + Zotero.getString(strPrefix + 'existingAttachments.plural',
-				newRelativeAttachmentIDs.length);
+			msg2 += Zotero.getString(chooseStrPrefix + 'existingAttachments.plural', numNewAttachments) + " ";
 	}
 	
+	switch (numOldAttachments) {
+		case 0:
+			break;
+		
+		case 1:
+			msg3 += Zotero.getString(clearStrPrefix + 'existingAttachments.singular');
+			break;
+		
+		default:
+			msg3 += Zotero.getString(clearStrPrefix + 'existingAttachments.plural', numOldAttachments);
+	}
+	
+	
 	var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING)
 		+ (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_CANCEL);
 	var index = ps.confirmEx(
 		null,
-		confirmTitle,
-		confirmString,
+		title,
+		(msg1 + msg2 + msg3).trim(),
 		buttonFlags,
-		Zotero.getString(strPrefix + 'button'),
+		Zotero.getString(chooseStrPrefix + 'button'),
 		null,
 		null,
 		null,
@@ -217,19 +274,21 @@ function chooseBaseAttachmentPath() {
 	
 	// Set new data directory
 	Zotero.debug("Setting new base directory");
-	Zotero.Prefs.set('baseAttachmentPath', basePathFile.persistentDescriptor);
+	Zotero.Prefs.set('baseAttachmentPath', newBasePathFile.persistentDescriptor);
 	Zotero.Prefs.set('saveRelativeAttachmentPath', true);
-	//Save all attachments on base path so that their paths become relative
-	var tempAttachment;
-	for (var index=0; index<newRelativeAttachmentIDs.length; index++) {
-		tempAttachment=Zotero.Items.get(newRelativeAttachmentIDs[index]);
-		if (!tempAttachment._changedAttachmentData) {
-			tempAttachment._changedAttachmentData = {};
+	// Resave all attachments on base path (so that their paths become relative)
+	// and all other relative attachments (so that their paths become absolute)
+	for (let id in newAttachmentPaths) {
+		let attachment = Zotero.Items.get(id);
+		if (newAttachmentPaths[id]) {
+			attachment.attachmentPath = newAttachmentPaths[id];
+			attachment.save();
+		}
+		else {
+			attachment.updateAttachmentPath();
 		}
-		tempAttachment._changedAttachmentData.path = true;
-		tempAttachment.save();
 	}
-	return basePathFile.path;
+	return newBasePathFile.persistentDescriptor;
 }
 
 function getBaseAttachmentPath(asFile) {
diff --git a/chrome/content/zotero/preferences/preferences.xul b/chrome/content/zotero/preferences/preferences.xul
index 4682b9bb8..cdac77516 100644
--- a/chrome/content/zotero/preferences/preferences.xul
+++ b/chrome/content/zotero/preferences/preferences.xul
@@ -871,5 +871,5 @@ To add a new preference:
 		observerService.notifyObservers(null, "charsetmenu-selected", "other");
 	]]>
 	</script>
-	<script src="preferences.js"/>
+	<script src="preferences.js" type="application/javascript;version=1.8"/>
 </prefwindow>
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 466b02e0d..c37766153 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -1522,21 +1522,37 @@ Zotero.Item.prototype.save = function() {
 				var path = this.attachmentPath;
 				var syncState = this.attachmentSyncState;
 				
-				//Save attachment in base attachment path as relative path
-				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
-					var basePath = Zotero.Prefs.get('baseAttachmentPath');
-					if (basePath!='' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
-						var baseDir = Components.classes["@mozilla.org/file/local;1"]
+				// Save attachment within attachment base directory as relative path
+				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE && path) {
+					let basePath = Zotero.Prefs.get('baseAttachmentPath');
+					if (basePath != '' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
+						let baseDir = Components.classes["@mozilla.org/file/local;1"]
 							.createInstance(Components.interfaces.nsILocalFile);
-						baseDir.initWithPath(basePath);
+						try {
+							baseDir.persistentDescriptor = basePath;
+						}
+						catch (e) {
+							Zotero.debug(e, 1);
+							Components.utils.reportError(e);
+							baseDir = null;
+						}
 						
-						var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-							.createInstance(Components.interfaces.nsILocalFile);
-						attachmentFile.initWithPath(path);
-						
-						if (baseDir.contains(attachmentFile,false)) {
-							path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
-								+attachmentFile.getRelativeDescriptor(baseDir);
+						if (baseDir) {
+							let attachmentFile = Components.classes["@mozilla.org/file/local;1"]
+								.createInstance(Components.interfaces.nsILocalFile);
+							try {
+								attachmentFile.initWithPath(path);
+							}
+							catch (e) {
+								Zotero.debug(e, 1);
+								Components.utils.reportError(e);
+								attachmentFile = null;
+							}
+							
+							if (attachmentFile && Zotero.File.directoryContains(baseDir, attachmentFile)) {
+								path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
+									+ attachmentFile.getRelativeDescriptor(baseDir);
+							}
 						}
 					}
 				}
@@ -1935,21 +1951,37 @@ Zotero.Item.prototype.save = function() {
 				var path = this.attachmentPath;
 				var syncState = this.attachmentSyncState;
 				
-				//Save attachment in base attachment path as relative path
-				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
-					var basePath = Zotero.Prefs.get('baseAttachmentPath');
-					if (basePath!='' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
-						var baseDir = Components.classes["@mozilla.org/file/local;1"]
+				// Save attachment within attachment base directory as relative path
+				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE && path) {
+					let basePath = Zotero.Prefs.get('baseAttachmentPath');
+					if (basePath != '' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
+						let baseDir = Components.classes["@mozilla.org/file/local;1"]
 							.createInstance(Components.interfaces.nsILocalFile);
-						baseDir.initWithPath(basePath);
+						try {
+							baseDir.persistentDescriptor = basePath;
+						}
+						catch (e) {
+							Zotero.debug(e, 1);
+							Components.utils.reportError(e);
+							baseDir = null;
+						}
 						
-						var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-							.createInstance(Components.interfaces.nsILocalFile);
-						attachmentFile.initWithPath(path);
-						
-						if (baseDir.contains(attachmentFile,false)) {
-							path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
-								+attachmentFile.getRelativeDescriptor(baseDir);
+						if (baseDir) {
+							let attachmentFile = Components.classes["@mozilla.org/file/local;1"]
+								.createInstance(Components.interfaces.nsILocalFile);
+							try {
+								attachmentFile.initWithPath(path);
+							}
+							catch (e) {
+								Zotero.debug(e, 1);
+								Components.utils.reportError(e);
+								attachmentFile = null;
+							}
+							
+							if (attachmentFile && Zotero.File.directoryContains(baseDir, attachmentFile)) {
+								path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
+									+ attachmentFile.getRelativeDescriptor(baseDir);
+							}
 						}
 					}
 				}
@@ -3238,16 +3270,22 @@ Zotero.Item.prototype.__defineGetter__('attachmentPath', function () {
 	}
 	
 	if (pathIsRelative) {
-		var baseDir = Components.classes["@mozilla.org/file/local;1"]
-			.createInstance(Components.interfaces.nsILocalFile);
 		var basePath = Zotero.Prefs.get('baseAttachmentPath');
-		if (basePath != '') {
-			baseDir.initWithPath(basePath);
-		} else {
-			//If the base path has been cleared, don't try to recreate the full attachment path
+		// If the base path has been cleared, don't try to recreate the full attachment path
+		if (basePath == '') {
 			return '';
 		}
-	
+		var baseDir = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			baseDir.persistentDescriptor = basePath;
+		}
+		catch (e) {
+			Zotero.debug(e, 1);
+			Components.utils.reportError(e);
+			return '';
+		}
+		
         var relativePath = this._attachmentPath.substr(
         	Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
 		var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
@@ -3285,6 +3323,21 @@ Zotero.Item.prototype.__defineSetter__('attachmentPath', function (val) {
 });
 
 
+/**
+ * Force an update of the attachment path and resave the item
+ *
+ * This is used when changing the attachment base directory, since relative
+ * path handling is done on item save.
+ */
+Zotero.Item.prototype.updateAttachmentPath = function () {
+	if (!this._changedAttachmentData) {
+		this._changedAttachmentData = {};
+	}
+	this._changedAttachmentData.path = true;
+	this.save();
+};
+
+
 Zotero.Item.prototype.__defineGetter__('attachmentSyncState', function () {
 	if (!this.isAttachment()) {
 		return undefined;
diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js
index 846f106ff..d07b1cd1a 100644
--- a/chrome/content/zotero/xpcom/file.js
+++ b/chrome/content/zotero/xpcom/file.js
@@ -268,6 +268,32 @@ Zotero.File = new function(){
 	}
 	
 	
+	/**
+	 * Check whether a directory is an ancestor directory of another directory/file
+	 */
+	this.directoryContains = function (dir, file) {
+		if (!dir.isDirectory()) {
+			throw new Error("dir must be a directory");
+		}
+		
+		if (dir.exists()) {
+			dir.normalize();
+		}
+		if (file.exists()) {
+			file.normalize();
+		}
+		
+		if (!dir.path) {
+			throw new Error("dir.path is empty");
+		}
+		if (!file.path) {
+			throw new Error("file.path is empty");
+		}
+		
+		return file.path.indexOf(dir.path) == 0;
+	}
+	
+	
 	/**
 	 * Strip potentially invalid characters
 	 *
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index 763ece7d8..0faa265ca 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -3396,6 +3396,7 @@ var ZoteroPane = new function()
 			}
 			
 			var file = item.getFile();
+			Zotero.debug("Opening " + file.path);
 			if (file) {
 				if(forceExternalViewer !== undefined) {
 					var externalViewer = forceExternalViewer;
diff --git a/chrome/locale/en-US/zotero/preferences.dtd b/chrome/locale/en-US/zotero/preferences.dtd
index ab65aebb3..1b3756e61 100644
--- a/chrome/locale/en-US/zotero/preferences.dtd
+++ b/chrome/locale/en-US/zotero/preferences.dtd
@@ -181,7 +181,7 @@
 <!ENTITY zotero.preferences.attachmentBaseDir.message        "Zotero will use relative paths for linked file attachments within the base directory, allowing you to access files on different computers as long as the file structure within the base directory remains the same.">
 <!ENTITY zotero.preferences.attachmentBaseDir.basePath       "Base directory:">
 <!ENTITY zotero.preferences.attachmentBaseDir.selectBasePath "Choose…">
-<!ENTITY zotero.preferences.attachmentBaseDir.resetBasePath  "Revert to absolute paths">
+<!ENTITY zotero.preferences.attachmentBaseDir.resetBasePath  "Revert to Absolute Paths…">
 
 <!ENTITY zotero.preferences.dbMaintenance							"Database Maintenance">
 <!ENTITY zotero.preferences.dbMaintenance.integrityCheck			"Check Database Integrity">
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
index fd76341d5..79713ed4c 100644
--- a/chrome/locale/en-US/zotero/zotero.properties
+++ b/chrome/locale/en-US/zotero/zotero.properties
@@ -69,17 +69,17 @@ errorReport.stepsToReproduce		= Steps to Reproduce:
 errorReport.expectedResult			= Expected result:
 errorReport.actualResult			= Actual result:
 
-attachmentBasePath.selectDir                                    = Choose Base Directory
-attachmentBasePath.confirmNewPath.title                         = Confirm new base directory
-attachmentBasePath.confirmNewPath.message                       = New linked file attachments below this directory will be saved using relative paths.
-attachmentBasePath.confirmNewPath.existingAttachments.singular  = One existing attachment within the new base directory will be converted to use a relative path.
-attachmentBasePath.confirmNewPath.existingAttachments.plural    = %S existing attachments within the new base directory will be converted to use relative paths.
-attachmentBasePath.confirmNewPath.button                        = Change Base Directory Setting
-attachmentBasePath.clearBasePath.title                          = Revert to absolute paths
-attachmentBasePath.clearBasePath.message                        = New linked file attachments will be saved using absolute paths.
-attachmentBasePath.clearBasePath.existingAttachments.singular   = One existing attachment within the old base directory will be converted to use an absolute path.
-attachmentBasePath.clearBasePath.existingAttachments.plural     = %S existing attachments within the old base directory will be converted to use absolute paths.
-attachmentBasePath.clearBasePath.button                         = Clear Base Directory Setting
+attachmentBasePath.selectDir                                   = Choose Base Directory
+attachmentBasePath.chooseNewPath.title                         = Confirm New Base Directory
+attachmentBasePath.chooseNewPath.message                       = Linked file attachments below this directory will be saved using relative paths.
+attachmentBasePath.chooseNewPath.existingAttachments.singular  = One existing attachment was found within the new base directory.
+attachmentBasePath.chooseNewPath.existingAttachments.plural    = %S existing attachments were found within the new base directory.
+attachmentBasePath.chooseNewPath.button                        = Change Base Directory Setting
+attachmentBasePath.clearBasePath.title                         = Revert to Absolute Paths
+attachmentBasePath.clearBasePath.message                       = New linked file attachments will be saved using absolute paths.
+attachmentBasePath.clearBasePath.existingAttachments.singular  = One existing attachment within the old base directory will be converted to use an absolute path.
+attachmentBasePath.clearBasePath.existingAttachments.plural    = %S existing attachments within the old base directory will be converted to use absolute paths.
+attachmentBasePath.clearBasePath.button                        = Clear Base Directory Setting
 
 dataDir.notFound						= The Zotero data directory could not be found.
 dataDir.previousDir					= Previous directory: