From dd1601793c2580b6430264e7f2884b5f2e5ec6f3 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Wed, 31 Jul 2024 01:39:25 -0400 Subject: [PATCH] Don't set default attachment title if not renaming file (#4459) Except from Rename File from Parent Metadata. --- chrome/content/zotero/xpcom/data/item.js | 11 +++++++---- chrome/content/zotero/zoteroPane.js | 6 ++++++ test/content/support.js | 4 +++- test/tests/attachmentsTest.js | 2 +- test/tests/zoteroPaneTest.js | 20 +++++++++++++------- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 65dbffb49b..3b84c6ec44 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3895,7 +3895,7 @@ Zotero.Item.prototype._getDefaultTitleForAttachmentContentType = function () { }; -Zotero.Item.prototype.setAutoAttachmentTitle = function () { +Zotero.Item.prototype.setAutoAttachmentTitle = function ({ ignoreAutoRenamePrefs } = {}) { if (!this.isAttachment()) { throw new Error("setAutoAttachmentTitle() can only be called on attachment items"); } @@ -3903,11 +3903,14 @@ Zotero.Item.prototype.setAutoAttachmentTitle = function () { return; } - // If this is the only attachment of its type on the parent item, give it - // a default title ("PDF", "Webpage", etc.) + // If this is the only attachment of its type on the parent item and the + // file is being renamed, give it a default title ("PDF", "Webpage", etc.) let isFirstOfType = this.parentItemID && this.parentItem.numFileAttachmentsWithContentType(this.attachmentContentType) <= 1; - if (isFirstOfType) { + let isBeingRenamed = ignoreAutoRenamePrefs + || (Zotero.Attachments.shouldAutoRenameFile(this.isLinkedFileAttachment()) + && Zotero.Attachments.isRenameAllowedForType(this.attachmentContentType)); + if (isFirstOfType && isBeingRenamed) { let defaultTitle = this._getDefaultTitleForAttachmentContentType(); if (defaultTitle !== null) { this.setField('title', defaultTitle); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index bf086ed77e..21522b0662 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -5667,6 +5667,7 @@ var ZoteroPane = new function() let parentItemID = item.parentItemID; let parentItem = await Zotero.Items.getAsync(parentItemID); + var oldBaseName = item.attachmentFilename.replace(/\.[^.]+$/, ''); var newName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: item.getField('title') }); let extRE = /\.[^\.]+$/; @@ -5682,6 +5683,11 @@ var ZoteroPane = new function() continue; } + if (item.getField('title') === oldBaseName) { + item.setAutoAttachmentTitle({ ignoreAutoRenamePrefs: true }); + await item.saveTx(); + } + let str = await document.l10n.formatValue('file-renaming-file-renamed-to', { filename: newName }); progressWin.addLines(str, item.getItemTypeIconName()); progressWin.show(); diff --git a/test/content/support.js b/test/content/support.js index de47ad61c1..1f1cecd92c 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -1007,7 +1007,9 @@ function importFileAttachment(filename, options = {}) { Object.assign(importOptions, options); // Override default behavior - don't set title based on type, // just use extension-less leafName - importOptions.title ??= file.leafName.replace(/\.[^.]+$/, ''); + if (!('title' in importOptions)) { + importOptions.title = file.leafName.replace(/\.[^.]+$/, ''); + } return Zotero.Attachments.importFromFile(importOptions); } diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index 80e4f4c954..c56619196e 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -152,7 +152,7 @@ describe("Zotero.Attachments", function() { file: file, parentItemID: parent.id, }); - assert.equal(attachment.getField('title'), Zotero.getString('file-type-pdf')); + assert.equal(attachment.getField('title'), 'test'); await parent.eraseTx(); }); }) diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index eb45552450..fe692e9d31 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -662,28 +662,34 @@ describe("ZoteroPane", function() { await item.saveTx(); var attachment = await importFileAttachment('test.png', { parentItemID: item.id }); - attachment.setField('title', 'Image'); + attachment.setField('title', 'Title'); await attachment.saveTx(); await zp.selectItem(attachment.id); await zp.renameSelectedAttachmentsFromParents(); assert.equal(attachment.attachmentFilename, 'Title.png'); - assert.equal(attachment.getField('title'), 'Image') + assert.equal(attachment.getField('title'), 'Title'); }); - it("should not change attachment title even if the same as filename", async function () { + it("should change attachment title if previously set to the file basename by setAutoAttachmentTitle()", async function () { var item = createUnsavedDataObject('item'); item.setField('title', 'Title'); await item.saveTx(); - var attachment = await importFileAttachment('test.png', { parentItemID: item.id }); - attachment.setField('title', 'test.png'); - await attachment.saveTx(); + var attachment = await importFileAttachment('test.png', { + parentItemID: item.id, + // Use default setAutoAttachmentTitle() behavior -- the file isn't going to be + // renamed because autoRenameFiles.fileTypes doesn't match image/, so the title + // becomes the filename minus extension, i.e., "test" + title: undefined + }); + assert.equal(attachment.getField('title'), 'test'); await zp.selectItem(attachment.id); await zp.renameSelectedAttachmentsFromParents(); assert.equal(attachment.attachmentFilename, 'Title.png'); - assert.equal(attachment.getField('title'), 'test.png') + // After a manual rename, the title becomes the default for this type + assert.equal(attachment.getField('title'), Zotero.getString('file-type-image')); }); });