diff --git a/chrome/content/zotero/preferences/preferences_file_renaming.js b/chrome/content/zotero/preferences/preferences_file_renaming.js index bab646cb43..b55c835c5b 100644 --- a/chrome/content/zotero/preferences/preferences_file_renaming.js +++ b/chrome/content/zotero/preferences/preferences_file_renaming.js @@ -50,9 +50,8 @@ Zotero_Preferences.FileRenaming = { if (selectedItem.isRegularItem() && !selectedItem.parentKey) { return [selectedItem, this.defaultExt, '']; } - if (selectedItem.isFileAttachment()) { - let path = selectedItem.getFilePath(); - let ext = Zotero.File.getExtension(Zotero.File.pathToFile(path)); + if (selectedItem.isFileAttachment() && selectedItem.parentKey) { + let ext = Zotero.Attachments.getCorrectFileExtension(selectedItem); let parentItem = Zotero.Items.getByLibraryAndKey(selectedItem.libraryID, selectedItem.parentKey); return [parentItem, ext ?? this.defaultExt, selectedItem.getField('title')]; } diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 855f410ca0..4ecd86eac0 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2213,7 +2213,7 @@ Zotero.Attachments = new function () { throw new Error("'item' must be a Zotero.Item"); } if (typeof options === 'string') { - Zotero.warn("Zotero.Attachments.getFileBaseNameFromItem(item, formatString) is deprecated -- use Zotero.Attachments(item, options)"); + Zotero.warn("Zotero.Attachments.getFileBaseNameFromItem(item, formatString) is deprecated -- use Zotero.Attachments.getFileBaseNameFromItem(item, options)"); options = { formatString: options }; } @@ -2463,7 +2463,24 @@ Zotero.Attachments = new function () { formatted = Zotero.File.getValidFileName(formatted); return formatted; }; - + + /** + * @returns {String} Current file extension for the attachment, if it appears to be a valid file extension. + * Otherwise, attempts to guess the file extension from the attachment's content type. + **/ + this.getCorrectFileExtension = function (attachment) { + let path = attachment.getFilePath(); + if (!path) { + return ''; + } + let ext = Zotero.File.getExtension(path); + ext = Zotero.File.isLikeExtension(ext) ? ext : ''; + if (ext === '') { + ext = Zotero.MIME.getPrimaryExtension(attachment.attachmentContentType); + Zotero.Debug.log(`Attachment "${path}": Invalid or missing extension. Guessing from content type: ${ext}`); + } + return ext; + }; this.shouldAutoRenameFile = function (isLink) { if (!Zotero.Prefs.get('autoRenameFiles')) { diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js index 607bf25e22..3214474094 100644 --- a/chrome/content/zotero/xpcom/file.js +++ b/chrome/content/zotero/xpcom/file.js @@ -83,6 +83,10 @@ Zotero.File = new function(){ var pos = file.leafName.lastIndexOf('.'); return pos==-1 ? '' : file.leafName.substr(pos+1); } + + this.isLikeExtension = function (extension) { + return !!extension.match(/^\w{1,10}$/i); + } /** diff --git a/chrome/content/zotero/xpcom/recognizeDocument.js b/chrome/content/zotero/xpcom/recognizeDocument.js index 4e590fb83c..c1115784c9 100644 --- a/chrome/content/zotero/xpcom/recognizeDocument.js +++ b/chrome/content/zotero/xpcom/recognizeDocument.js @@ -294,8 +294,8 @@ Zotero.RecognizeDocument = new function () { // Rename attachment file to match new metadata if (Zotero.Attachments.shouldAutoRenameAttachment(attachment)) { - let ext = Zotero.File.getExtension(path); let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: originalTitle }); + let ext = Zotero.Attachments.getCorrectFileExtension(attachment); let newName = fileBaseName + (ext ? '.' + ext : ''); let result = await attachment.renameAttachmentFile(newName, false, true); if (result !== true) { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 877ba71310..d506c268af 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -5624,8 +5624,8 @@ var ZoteroPane = new function() Zotero.debug('No path for attachment ' + item.key); continue; } - let ext = Zotero.File.getExtension(path); let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(item.parentItem, { attachmentTitle: item.getField('title') }); + let ext = Zotero.Attachments.getCorrectFileExtension(item); let newName = fileBaseName + (ext ? '.' + ext : ''); let result = await item.renameAttachmentFile(newName, false, true); if (result !== true) { @@ -5955,14 +5955,9 @@ 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 = /\.[^\.]+$/; - let origFilename = PathUtils.split(file).pop(); - let ext = origFilename.match(extRE); - if (ext) { - newName = newName + ext[0]; - } + var fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: item.getField('title') }); + let ext = Zotero.Attachments.getCorrectFileExtension(item); + let newName = fileBaseName + (ext ? '.' + ext : ''); var renamed = await item.renameAttachmentFile(newName, false, true); if (renamed !== true) { diff --git a/test/tests/recognizeDocumentTest.js b/test/tests/recognizeDocumentTest.js index ee22d35201..d6dcd6fa2b 100644 --- a/test/tests/recognizeDocumentTest.js +++ b/test/tests/recognizeDocumentTest.js @@ -323,6 +323,35 @@ describe("Document Recognition", function() { 'test' ); }); + + it("should use mime-type-derived extension for files with invalid extensions", async function () { + if (Zotero.automatedTest) { + this.skip(); // TODO: Mock services + } + this.timeout(30000); + let pdfFile = getTestDataDirectory(); + pdfFile.append('recognizePDF_test_title.pdf'); + let tmpDir = await getTempDirectory(); + let tmpFileToImport = OS.Path.join(tmpDir, 'bad name . not an extension'); + await OS.File.copy(pdfFile.path, tmpFileToImport); + var attachment = await Zotero.Attachments.importFromFile({ file: tmpFileToImport }); + + win.ZoteroPane.recognizeSelected(); + + var addedIDs = await waitForItemEvent("add"); + var modifiedIDs = await waitForItemEvent("modify"); + assert.lengthOf(addedIDs, 1); + var item = Zotero.Items.get(addedIDs[0]); + assert.lengthOf(modifiedIDs, 2); + + await waitForProgressWindow(); + + // The file should have been renamed + assert.equal( + attachment.attachmentFilename, + Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf' + ); + }); }); describe("Ebooks", function () { diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 22b8658ddb..203e0da309 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -629,8 +629,8 @@ describe("ZoteroPane", function() { it("should use unique name for linked file without extension if target name is taken", async function () { var oldFilename = 'old'; - var newFilename = 'Test'; - var uniqueFilename = 'Test 2'; + var newFilename = 'Test.png'; + var uniqueFilename = 'Test 2.png'; var file = getTestDataDirectory(); file.append('test.png'); var tmpDir = await getTempDirectory(); @@ -652,7 +652,7 @@ describe("ZoteroPane", function() { await zp.renameSelectedAttachmentsFromParents(); assert.equal(attachment.attachmentFilename, uniqueFilename); var path = await attachment.getFilePathAsync(); - assert.equal(OS.Path.basename(path), uniqueFilename) + assert.equal(OS.Path.basename(path), uniqueFilename); await OS.File.exists(path); }); @@ -691,6 +691,27 @@ describe("ZoteroPane", function() { // After a manual rename, the title becomes the default for this type assert.equal(attachment.getField('title'), Zotero.getString('file-type-image')); }); + + it("should restore an extension when renaming a misnamed file", async function () { + let pdfFile = getTestDataDirectory(); + pdfFile.append('test.pdf'); + let tmpDir = await getTempDirectory(); + let tmpFileToImport = OS.Path.join(tmpDir, 'bad name . not an extension'); + await OS.File.copy(pdfFile.path, tmpFileToImport); + + var item = createUnsavedDataObject('item'); + item.setField('title', 'Title'); + await item.saveTx(); + + let attachment = await Zotero.Attachments.importFromFile({ + file: tmpFileToImport, + parentItemID: item.id + }); + + await zp.selectItem(attachment.id); + await zp.renameSelectedAttachmentsFromParents(); + assert.equal(attachment.attachmentFilename, 'Title.pdf'); + }); });