From 7ea14aa34ebbfa98861d4e61faa2b1f5eb3c634a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 10 Sep 2020 04:34:10 -0400 Subject: [PATCH] Add Zotero.Item::isStoredFileAttachment() And replace most uses of isImportedAttachment(), which doesn't include embedded-image attachments There might not be much reason to keep isImportedAttachment() around. --- chrome/content/zotero/xpcom/attachments.js | 10 +++--- chrome/content/zotero/xpcom/data/item.js | 36 ++++++++++--------- chrome/content/zotero/xpcom/sync/syncLocal.js | 6 ++-- chrome/content/zotero/zoteroPane.js | 4 +-- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 470e6a8a12..9cbdae1b8a 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2447,7 +2447,7 @@ Zotero.Attachments = new function(){ Zotero.DB.requireTransaction(); var newAttachment = attachment.clone(libraryID); - if (attachment.isImportedAttachment()) { + if (attachment.isStoredFileAttachment()) { // Attachment path isn't copied over by clone() if libraryID is different newAttachment.attachmentPath = attachment.attachmentPath; } @@ -2459,7 +2459,7 @@ Zotero.Attachments = new function(){ // Move files over if they exist var oldDir; var newDir; - if (newAttachment.isImportedAttachment()) { + if (newAttachment.isStoredFileAttachment()) { oldDir = this.getStorageDirectory(attachment).path; if (await OS.File.exists(oldDir)) { newDir = this.getStorageDirectory(newAttachment).path; @@ -2485,7 +2485,7 @@ Zotero.Attachments = new function(){ } catch (e) { // Move files back if old item can't be deleted - if (newAttachment.isImportedAttachment()) { + if (newAttachment.isStoredFileAttachment()) { try { await OS.File.move(newDir, oldDir); } @@ -2511,7 +2511,7 @@ Zotero.Attachments = new function(){ Zotero.DB.requireTransaction(); var newAttachment = attachment.clone(libraryID); - if (attachment.isImportedAttachment()) { + if (attachment.isStoredFileAttachment()) { // Attachment path isn't copied over by clone() if libraryID is different newAttachment.attachmentPath = attachment.attachmentPath; } @@ -2521,7 +2521,7 @@ Zotero.Attachments = new function(){ yield newAttachment.save(); // Copy over files if they exist - if (newAttachment.isImportedAttachment() && (yield attachment.fileExists())) { + if (newAttachment.isStoredFileAttachment() && (yield attachment.fileExists())) { let dir = Zotero.Attachments.getStorageDirectory(attachment); let newDir = yield Zotero.Attachments.createDirectoryForItem(newAttachment); yield Zotero.File.copyDirectory(dir, newDir); diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 8e489e59c1..4e738644a3 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -2233,12 +2233,21 @@ Zotero.Item.prototype.isImportedAttachment = function() { switch (linkMode) { case Zotero.Attachments.LINK_MODE_IMPORTED_FILE: case Zotero.Attachments.LINK_MODE_IMPORTED_URL: - case Zotero.Attachments.LINK_MODE_EMBEDDED_IMAGE: return true; } return false; } +/** + * @return {Promise} + */ +Zotero.Item.prototype.isStoredFileAttachment = function() { + if (!this.isAttachment()) { + return false; + } + return this.isImportedAttachment() || this.isEmbeddedImageAttachment(); +} + /** * @return {Promise} */ @@ -2443,7 +2452,7 @@ Zotero.Item.prototype.getFilePathAsync = Zotero.Promise.coroutine(function* () { } // Imported file with relative path - if (this.isImportedAttachment()) { + if (this.isStoredFileAttachment()) { if (!path.includes("storage:")) { Zotero.logError("Invalid attachment path '" + path + "'"); this._updateAttachmentStates(false); @@ -2628,7 +2637,7 @@ Zotero.Item.prototype.renameAttachmentFile = async function (newName, overwrite try { let origName = OS.Path.basename(origPath); - if (this.isImportedAttachment()) { + if (this.isStoredFileAttachment()) { var origModDate = (await OS.File.stat(origPath)).lastModificationDate; } @@ -2641,7 +2650,7 @@ Zotero.Item.prototype.renameAttachmentFile = async function (newName, overwrite // Update mod time and clear hash so the file syncs // TODO: use an integer counter instead of mod time for change detection // Update mod time first, because it may fail for read-only files on Windows - if (this.isImportedAttachment()) { + if (this.isStoredFileAttachment()) { await OS.File.setDates(origPath, null, null); } @@ -2657,7 +2666,7 @@ Zotero.Item.prototype.renameAttachmentFile = async function (newName, overwrite await this.relinkAttachmentFile(destPath); - if (this.isImportedAttachment()) { + if (this.isStoredFileAttachment()) { this.attachmentSyncedHash = null; this.attachmentSyncState = "to_upload"; await this.saveTx({ skipAll: true }); @@ -2669,7 +2678,7 @@ Zotero.Item.prototype.renameAttachmentFile = async function (newName, overwrite Zotero.logError(e); // Restore original modification date in case we managed to change it - if (this.isImportedAttachment()) { + if (this.isStoredFileAttachment()) { try { OS.File.setDates(origPath, null, origModDate); } catch (e) { @@ -2714,7 +2723,7 @@ Zotero.Item.prototype.relinkAttachmentFile = Zotero.Promise.coroutine(function* // 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).path; - if (this.isImportedAttachment() && OS.Path.dirname(path) != storageDir) { + if (this.isStoredFileAttachment() && OS.Path.dirname(path) != storageDir) { newPath = OS.Path.join(storageDir, newName); // If file with same name already exists in the storage directory, @@ -3105,13 +3114,8 @@ Zotero.defineProperty(Zotero.Item.prototype, 'attachmentSyncState', { val = Zotero.Sync.Storage.Local["SYNC_STATE_" + val.toUpperCase()]; } - switch (this.attachmentLinkMode) { - case Zotero.Attachments.LINK_MODE_IMPORTED_URL: - case Zotero.Attachments.LINK_MODE_IMPORTED_FILE: - break; - - default: - throw new Error("attachmentSyncState can only be set for stored files"); + if (!this.isStoredFileAttachment()) { + throw new Error("attachmentSyncState can only be set for stored files"); } switch (val) { @@ -4493,7 +4497,7 @@ Zotero.Item.prototype._eraseData = Zotero.Promise.coroutine(function* (env) { } // Zotero.Sync.EventListeners.ChangeListener needs to know if this was a storage file - env.notifierData[this.id].storageDeleteLog = this.isImportedAttachment(); + env.notifierData[this.id].storageDeleteLog = this.isStoredFileAttachment(); } // Regular item else { @@ -4933,7 +4937,7 @@ Zotero.Item.prototype.toJSON = function (options = {}) { obj.filename = this.attachmentFilename; } - if (this.isImportedAttachment() && !options.skipStorageProperties) { + if (this.isStoredFileAttachment() && !options.skipStorageProperties) { if (options.syncedStorageProperties) { let mtime = this.attachmentSyncedModificationTime; // There's never a reason to include these if they're null. This can happen if diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 6952aaf015..f7ac3e1a4e 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -883,7 +883,7 @@ Zotero.Sync.Data.Local = { // For items, check if mtime or file hash changed in metadata, // which would indicate that a remote storage sync took place and // a download is needed - if (objectType == 'item' && obj.isImportedAttachment()) { + if (objectType == 'item' && obj.isStoredFileAttachment()) { if (jsonDataLocal.mtime != jsonData.mtime || jsonDataLocal.md5 != jsonData.md5) { saveOptions.storageDetailsChanged = true; @@ -1473,7 +1473,7 @@ Zotero.Sync.Data.Local = { } } - if (obj.isImportedAttachment()) { + if (obj.isStoredFileAttachment()) { yield this._checkAttachmentForDownload(obj, json.data.mtime, options.isNewObject); } } @@ -1508,7 +1508,7 @@ Zotero.Sync.Data.Local = { yield this._removeObjectFromSyncQueue(obj.objectType, obj.libraryID, json.key); // Mark updated attachments for download - if (obj.objectType == 'item' && obj.isImportedAttachment()) { + if (obj.objectType == 'item' && obj.isStoredFileAttachment()) { // If storage changes were made (attachment mtime or hash), mark // library as requiring download if (options.isNewObject || options.storageDetailsChanged) { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 2d249cfa9c..449efc7c7e 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -4089,7 +4089,7 @@ var ZoteroPane = new function() continue; } - let isLinkedFile = !item.isImportedAttachment(); + let isLinkedFile = !item.isStoredFileAttachment(); let path = item.getFilePath(); if (!path) { ZoteroPane_Local.showAttachmentNotFoundDialog( @@ -4265,7 +4265,7 @@ var ZoteroPane = new function() var fileExists = await OS.File.exists(path); // If file doesn't exist but an evicted iCloud Drive file does, reveal that instead - if (!fileExists && Zotero.isMac && !attachment.isImportedAttachment()) { + if (!fileExists && Zotero.isMac && !attachment.isStoredFileAttachment()) { let iCloudPath = Zotero.File.getEvictedICloudPath(path); if (await OS.File.exists(iCloudPath)) { path = iCloudPath;