commit dfb28ff3f675157b7ef2a6c4fe92320f2f81cc38
parent 0511d37b07b80cdf67131342642089d7aee13bb4
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 24 Jun 2015 05:38:41 -0400
File Issues: Greatest Hits, 2006-2015
- When relinking a missing stored file, copy it into the attachment's
storage directory automatically
- Previously, selecting a file outside the attachment subdir would
just result in a missing attachment, since it only looks for stored
files within the subdir
- Display an error message if a Windows shortcut (.lnk) is added via
drag-and-drop or via a file dialog on non-Windows systems, until we
can figure out how to determine the original file
- Shortcuts can cause errors during syncing, for unclear reasons
- Neither nsIFile::copyToFollowingLinks() nor nsIFile::target work for
me to get the original file, even when nsIFile::isSymlink() returns
true
- Windows file dialogs seem to automatically resolve shortcuts, so
it's only an issue there for drag-and-drop
- Disallow hidden files from being selected in relink dialog
- I think some people on Windows with hidden files shown relink the
.zotero* files that show up when they click Locate, which causes
file sync errors. Which brings us to...
- Fix file sync errors for *.lnk and .zotero* files
- Ignore existing .zotero* attachment files, treating the files as
missing instead to encourage relinking
- Strip leading period in getValidFileName() to prevent added files from
being hidden
- This allows hidden files to be added explicitly; they just won't
stay that way in the storage directory
(These things should have tests, but that will have to happen on the 5.0
branch.)
Diffstat:
8 files changed, 128 insertions(+), 34 deletions(-)
diff --git a/chrome/content/zotero/webpagedump/common.js b/chrome/content/zotero/webpagedump/common.js
@@ -644,7 +644,7 @@ var wpdCommon = {
aDir.initWithPath(destdir);
- aFile.copyTo(aDir, destfile);
+ aFile.copyToFollowingLinks(aDir, destfile);
return true; // Added by Dan S. for Zotero
},
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
@@ -55,6 +55,10 @@ Zotero.Attachments = new function(){
throw ("'" + file.leafName + "' must be a file in Zotero.Attachments.importFromFile()");
}
+ if (file.leafName.endsWith(".lnk")) {
+ throw new Error("Cannot add Windows shortcut");
+ }
+
Zotero.DB.beginTransaction();
try {
@@ -165,7 +169,7 @@ Zotero.Attachments = new function(){
var storageDir = Zotero.getStorageDirectory();
var destDir = this.getStorageDirectory(itemID);
_moveOrphanedDirectory(destDir);
- file.parent.copyTo(storageDir, destDir.leafName);
+ file.parent.copyToFollowingLinks(storageDir, destDir.leafName);
// Point to copied file
var newFile = destDir.clone();
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -2863,6 +2863,11 @@ Zotero.Item.prototype.getFile = function(row, skipExistsCheck) {
if (Zotero.isWin || path.indexOf('/') != -1) {
path = Zotero.File.getValidFileName(path, true);
}
+ // Ignore .zotero* files that were relinked before we started blocking them
+ if (path.startsWith(".zotero")) {
+ Zotero.debug("Ignoring attachment file " + path, 2);
+ return false;
+ }
var file = Zotero.Attachments.getStorageDirectory(this.id);
file.QueryInterface(Components.interfaces.nsILocalFile);
file.setRelativeDescriptor(file, path);
@@ -3109,15 +3114,47 @@ Zotero.Item.prototype.relinkAttachmentFile = function(file, skipItemUpdate) {
throw('Cannot relink linked URL in Zotero.Items.relinkAttachmentFile()');
}
+ if (file.leafName.endsWith(".lnk")) {
+ throw new Error("Cannot relink to Windows shortcut");
+ }
+
var newName = Zotero.File.getValidFileName(file.leafName);
if (!newName) {
throw ("No valid characters in filename after filtering in Zotero.Item.relinkAttachmentFile()");
}
- // Rename file to filtered name if necessary
- if (file.leafName != newName) {
- Zotero.debug("Renaming file '" + file.leafName + "' to '" + newName + "'");
- file.moveTo(null, newName);
+ try {
+ // If selected file isn't in the attachment's storage directory,
+ // copy it in and use that one instead
+ var storageDir = Zotero.Attachments.getStorageDirectory(this.id);
+ if (this.isImportedAttachment() && !file.parent.equals(storageDir)) {
+ // If file with same name already exists in the storage directory,
+ // move it out of the way
+ let targetFile = storageDir.clone();
+ targetFile.append(newName);
+ let renamedFile;
+ if (targetFile.exists()) {
+ renamedFile = targetFile.clone();
+ renamedFile.moveTo(null, newName + ".bak");
+ }
+
+ Zotero.File.copyToUnique(file, targetFile);
+ file = targetFile;
+
+ // Delete backup file
+ if (renamedFile) {
+ renamedFile.remove(false);
+ }
+ }
+ // Rename file to filtered name if necessary
+ else if (file.leafName != newName) {
+ Zotero.debug("Renaming file '" + file.leafName + "' to '" + newName + "'");
+ file.moveTo(null, newName);
+ }
+ }
+ catch (e) {
+ Zotero.logError(e);
+ return false;
}
var path = Zotero.Attachments.getPath(file, linkMode);
@@ -3128,7 +3165,7 @@ Zotero.Item.prototype.relinkAttachmentFile = function(file, skipItemUpdate) {
skipClientDateModifiedUpdate: skipItemUpdate
});
- return false;
+ return true;
}
@@ -3342,7 +3379,8 @@ Zotero.Item.prototype.__defineGetter__('attachmentPath', function () {
var sql = "SELECT path FROM itemAttachments WHERE itemID=?";
var path = Zotero.DB.valueQuery(sql, this.id);
- if (!path) {
+ // Ignore .zotero* files that were relinked before we started blocking them
+ if (!path || path.startsWith('.zotero')) {
path = '';
}
this._attachmentPath = path;
diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js
@@ -426,7 +426,7 @@ Zotero.File = new function(){
newFile.remove(null);
// Copy file to unique name
- file.copyTo(newFile.parent, newName);
+ file.copyToFollowingLinks(newFile.parent, newName);
return newFile;
}
@@ -442,7 +442,7 @@ Zotero.File = new function(){
while (otherFiles.hasMoreElements()) {
var file = otherFiles.getNext();
file.QueryInterface(Components.interfaces.nsIFile);
- file.copyTo(newDir, null);
+ file.copyToFollowingLinks(newDir, null);
}
}
@@ -505,6 +505,8 @@ Zotero.File = new function(){
// Strip characters not valid in XML, since they won't sync and they're probably unwanted
fileName = fileName.replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\ud800-\udfff\ufffe\uffff]/g, '');
}
+ // Don't allow hidden files
+ fileName = fileName.replace(/^\./, '');
// Don't allow blank or illegal filenames
if (!fileName || fileName == '.' || fileName == '..') {
fileName = '_';
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -2810,7 +2810,7 @@ Zotero.ItemTreeView.fileDragDataProvider.prototype = {
}
}
- parentDir.copyTo(destDir, newName ? newName : dirName);
+ parentDir.copyToFollowingLinks(destDir, newName ? newName : dirName);
// Store nsIFile
if (useTemp) {
@@ -2851,7 +2851,7 @@ Zotero.ItemTreeView.fileDragDataProvider.prototype = {
}
}
- file.copyTo(destDir, newName ? newName : null);
+ file.copyToFollowingLinks(destDir, newName ? newName : null);
// Store nsIFile
if (useTemp) {
@@ -3219,6 +3219,14 @@ Zotero.ItemTreeView.prototype.drop = function(row, orient, dataTransfer) {
var itemID = Zotero.Attachments.linkFromFile(file, sourceItemID);
}
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);
+ Zotero.DB.commitTransaction();
+ continue;
+ }
var itemID = Zotero.Attachments.importFromFile(file, sourceItemID, targetLibraryID);
// If moving, delete original file
if (dragData.dropEffect == 'move') {
diff --git a/chrome/content/zotero/xpcom/storage.js b/chrome/content/zotero/xpcom/storage.js
@@ -1486,6 +1486,10 @@ Zotero.Sync.Storage = new function () {
if (!file) {
throw ("Empty path for item " + item.key + " in " + funcName);
}
+ // Don't save Windows aliases
+ if (file.leafName.endsWith('.lnk')) {
+ return false;
+ }
var fileName = file.leafName;
var renamed = false;
@@ -1899,8 +1903,9 @@ Zotero.Sync.Storage = new function () {
params.push(Zotero.Sync.Storage.SYNC_STATE_TO_DOWNLOAD);
}
sql += ") "
- // Skip attachments with empty path, which can't be saved
- + "AND path!=''";
+ // Skip attachments with empty path, which can't be saved, and files with .zotero*
+ // paths, which have somehow ended up in some users' libraries
+ + "AND path!='' AND path NOT LIKE 'storage:.zotero%'";
var itemIDs = Zotero.DB.columnQuery(sql, params);
if (!itemIDs) {
return [];
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
@@ -3116,8 +3116,16 @@ var ZoteroPane = new function()
var attachmentID;
if(link)
attachmentID = Zotero.Attachments.linkFromFile(file, id);
- else
+ 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;
+ }
attachmentID = Zotero.Attachments.importFromFile(file, id, libraryID);
+ }
if(attachmentID && !id)
{
@@ -3757,6 +3765,16 @@ var ZoteroPane = new function()
}
+ // TODO: Figure out a functioning way to get the original path and just copy the real file
+ this.displayCannotAddShortcutMessage = function (path) {
+ Zotero.alert(
+ null,
+ Zotero.getString("general.error"),
+ Zotero.getString("file.error.cannotAddShortcut") + (path ? "\n\n" + path : "")
+ );
+ }
+
+
function showAttachmentNotFoundDialog(itemID, noLocate) {
var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].
createInstance(Components.interfaces.nsIPromptService);
@@ -3951,25 +3969,43 @@ var ZoteroPane = new function()
throw('Item ' + itemID + ' not found in ZoteroPane_Local.relinkAttachment()');
}
- var nsIFilePicker = Components.interfaces.nsIFilePicker;
- var fp = Components.classes["@mozilla.org/filepicker;1"]
- .createInstance(nsIFilePicker);
- fp.init(window, Zotero.getString('pane.item.attachments.select'), nsIFilePicker.modeOpen);
-
-
- var file = item.getFile(false, true);
- var dir = Zotero.File.getClosestDirectory(file);
- if (dir) {
- dir.QueryInterface(Components.interfaces.nsILocalFile);
- fp.displayDirectory = dir;
- }
-
- fp.appendFilters(Components.interfaces.nsIFilePicker.filterAll);
-
- if (fp.show() == nsIFilePicker.returnOK) {
- var file = fp.file;
- file.QueryInterface(Components.interfaces.nsILocalFile);
- item.relinkAttachmentFile(file);
+ while (true) {
+ var nsIFilePicker = Components.interfaces.nsIFilePicker;
+ var fp = Components.classes["@mozilla.org/filepicker;1"]
+ .createInstance(nsIFilePicker);
+ fp.init(window, Zotero.getString('pane.item.attachments.select'), nsIFilePicker.modeOpen);
+
+
+ var file = item.getFile(false, true);
+ var dir = Zotero.File.getClosestDirectory(file);
+ if (dir) {
+ dir.QueryInterface(Components.interfaces.nsILocalFile);
+ fp.displayDirectory = dir;
+ }
+
+ fp.appendFilters(Components.interfaces.nsIFilePicker.filterAll);
+
+ if (fp.show() == nsIFilePicker.returnOK) {
+ let file = fp.file;
+ file.QueryInterface(Components.interfaces.nsILocalFile);
+
+ // Disallow hidden files
+ // TODO: Display a message
+ if (file.leafName.startsWith('.')) {
+ continue;
+ }
+
+ // Disallow Windows shortcuts
+ if (file.leafName.endsWith(".lnk")) {
+ this.displayCannotAddShortcutMessage(file.path);
+ continue;
+ }
+
+ item.relinkAttachmentFile(file);
+ break;
+ }
+
+ break;
}
}
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
@@ -963,6 +963,7 @@ file.accessError.message.windows = Check that the file is not currently in use,
file.accessError.message.other = Check that the file is not currently in use and that its permissions allow write access.
file.accessError.restart = Restarting your computer or disabling security software may also help.
file.accessError.showParentDir = Show Parent Directory
+file.error.cannotAddShortcut = Shortcut files cannot be added directly. Please select the original file.
lookup.failure.title = Lookup Failed
lookup.failure.description = Zotero could not find a record for the specified identifier. Please verify the identifier and try again.
