diff --git a/chrome/content/zotero/preferences/preferences_general.xul b/chrome/content/zotero/preferences/preferences_general.xul
index 2ef9b2814b..fc0c087e8f 100644
--- a/chrome/content/zotero/preferences/preferences_general.xul
+++ b/chrome/content/zotero/preferences/preferences_general.xul
@@ -37,6 +37,7 @@
+
@@ -122,6 +123,7 @@
label="&zotero.preferences.automaticSnapshots;"
preference="pref-automaticSnapshots"/>
+
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index 68a3e1b5cf..c50cfdd0e3 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -245,9 +245,18 @@ Zotero.Attachments = new function(){
/**
- * @param {Object} options - 'libraryID', 'url', 'parentItemID', 'collections', 'title',
- * 'fileBaseName', 'contentType', 'referrer', 'cookieSandbox',
- * 'saveOptions'
+ * @param {Object} options
+ * @param {Integer} options.libraryID
+ * @param {String} options.url
+ * @param {Integer} [options.parentItemID]
+ * @param {Integer[]} [options.collections]
+ * @param {String} [options.title]
+ * @param {String} [options.fileBaseName]
+ * @param {Boolean} [options.renameIfAllowedType=false]
+ * @param {String} [options.contentType]
+ * @param {String} [options.referrer]
+ * @param {CookieSandbox} [options.cookieSandbox]
+ * @param {Object} [options.saveOptions]
* @return {Promise} - A promise for the created attachment item
*/
this.importFromURL = Zotero.Promise.coroutine(function* (options) {
@@ -257,6 +266,7 @@ Zotero.Attachments = new function(){
var collections = options.collections;
var title = options.title;
var fileBaseName = options.fileBaseName;
+ var renameIfAllowedType = options.renameIfAllowedType;
var contentType = options.contentType;
var referrer = options.referrer;
var cookieSandbox = options.cookieSandbox;
@@ -320,6 +330,11 @@ Zotero.Attachments = new function(){
// Save using remote web browser persist
var externalHandlerImport = Zotero.Promise.coroutine(function* (contentType) {
+ // Rename attachment
+ if (renameIfAllowedType && !fileBaseName && this.getRenamedFileTypes().includes(contentType)) {
+ let parentItem = Zotero.Items.get(parentItemID);
+ fileBaseName = this.getFileBaseNameFromItem(parentItem);
+ }
if (fileBaseName) {
let ext = _getExtensionFromURL(url, contentType);
var fileName = fileBaseName + (ext != '' ? '.' + ext : '');
@@ -804,6 +819,30 @@ Zotero.Attachments = new function(){
}
+ this.getRenamedFileTypes = function () {
+ try {
+ var types = Zotero.Prefs.get('renameAttachmentFiles.automatic.fileTypes');
+ return types ? types.split(',') : [];
+ }
+ catch (e) {
+ return [];
+ }
+ };
+
+
+ this.getRenamedFileBaseNameIfAllowedType = async function (parentItem, file) {
+ var types = this.getRenamedFileTypes();
+ var contentType = file.endsWith('.pdf')
+ // Don't bother reading file if there's a .pdf extension
+ ? 'application/pdf'
+ : await Zotero.MIME.getMIMETypeFromFile(file);
+ if (!types.includes(contentType)) {
+ return false;
+ }
+ return this.getFileBaseNameFromItem(parentItem);
+ }
+
+
/**
* Create directory for attachment files within storage directory
*
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 76828d42ae..023bd1cdcc 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -2120,6 +2120,15 @@ Zotero.Item.prototype.numAttachments = function (includeTrashed) {
}
+Zotero.Item.prototype.numNonHTMLFileAttachments = function () {
+ this._requireData('childItems');
+ return this.getAttachments()
+ .map(itemID => Zotero.Items.get(itemID))
+ .filter(item => item.isFileAttachment() && item.attachmentContentType != 'text/html')
+ .length;
+};
+
+
Zotero.Item.prototype.getFile = function () {
Zotero.debug("Zotero.Item.prototype.getFile() is deprecated -- use getFilePath[Async]()", 2);
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 80aff7a6e8..0bb7021125 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -3231,26 +3231,22 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
var notifierQueue = new Zotero.Notifier.Queue;
try {
+ // If there's a single file being added to a parent, automatic renaming is enabled,
+ // and there are no other non-HTML attachments, we'll rename the file as long as it's
+ // an allowed type. The dragged data could be a URL, so we don't yet know the file type.
+ // This should be kept in sync with ZoteroPane.addAttachmentFromDialog().
+ let renameIfAllowedType = false;
let parentItem;
- let numExistingFileAttachments;
- if (parentItemID) {
+ if (parentItemID && data.length == 1 && Zotero.Prefs.get('renameAttachmentFiles.automatic')) {
parentItem = Zotero.Items.get(parentItemID);
- numExistingFileAttachments = parentItem.getAttachments()
- .map(itemID => Zotero.Items.get(itemID))
- .filter(item => item.isFileAttachment())
- .length;
+ if (!parentItem.numNonHTMLFileAttachments()) {
+ renameIfAllowedType = true;
+ }
}
for (var i=0; i
+
diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js
index 4946ddeef2..38760ba7f4 100644
--- a/defaults/preferences/zotero.js
+++ b/defaults/preferences/zotero.js
@@ -35,6 +35,8 @@ pref("extensions.zotero.automaticTags",true);
pref("extensions.zotero.fontSize", "1.0");
pref("extensions.zotero.layout", "standard");
pref("extensions.zotero.recursiveCollections", false);
+pref("extensions.zotero.renameAttachmentFiles.automatic", true);
+pref("extensions.zotero.renameAttachmentFiles.automatic.fileTypes", "application/pdf");
pref("extensions.zotero.attachmentRenameFormatString", '{%c - }{%y - }{%t{50}}');
pref("extensions.zotero.capitalizeTitles", false);
pref("extensions.zotero.launchNonNativeFiles", false);
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index 3f474892d1..ca485efb49 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -695,6 +695,12 @@ describe("Zotero.ItemTreeView", function() {
file.append(pdfFilename);
pdfPath = file.path;
httpd.registerFile("/" + pdfFilename, file);
+
+ Zotero.Prefs.clear('renameAttachmentFiles.automatic');
+ });
+
+ afterEach(() => {
+ Zotero.Prefs.clear('renameAttachmentFiles.automatic');
});
after(function* () {
@@ -781,7 +787,7 @@ describe("Zotero.ItemTreeView", function() {
assert.isFalse(view.isContainerEmpty(view.getRowIndexByID(item1.id)));
});
- it("should create a top-level attachment when a file is dragged", function* () {
+ it("should create a stored top-level attachment when a file is dragged", function* () {
var file = getTestDataDirectory();
file.append('test.png');
@@ -812,7 +818,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
- it("should create a top-level attachment when a URL is dragged", function* () {
+ it("should create a stored top-level attachment when a URL is dragged", function* () {
var promise = itemsView.waitForSelect();
itemsView.drop(0, -1, {
@@ -840,7 +846,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
- it("should create a child attachment when a URL is dragged", function* () {
+ it("should create a stored child attachment when a URL is dragged", function* () {
var view = zp.itemsView;
var parentItem = yield createDataObject('item');
var parentRow = view.getRowIndexByID(parentItem.id);
@@ -873,7 +879,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
- it("should rename a child attachment using parent metadata if no existing file attachments", async function () {
+ it("should rename a stored child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
@@ -885,7 +891,7 @@ describe("Zotero.ItemTreeView", function() {
var parentRow = view.getRowIndexByID(parentItem.id);
var file = getTestDataDirectory();
- file.append('test.png');
+ file.append('empty.pdf');
var promise = waitForItemEvent('add');
@@ -910,11 +916,101 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
- assert.equal(title, parentTitle + '.png');
- assert.equal(OS.Path.basename(path), parentTitle + '.png');
+ assert.equal(title, parentTitle + '.pdf');
+ assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
});
- it("shouldn't rename a child attachment using parent metadata if existing file attachments", async function () {
+ it("should rename a linked child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
+ var view = zp.itemsView;
+ var parentTitle = Zotero.Utilities.randomString();
+ var parentItem = await createDataObject('item', { title: parentTitle });
+ await Zotero.Attachments.linkFromURL({
+ url: 'https://example.com',
+ title: 'Example',
+ parentItemID: parentItem.id
+ });
+ var parentRow = view.getRowIndexByID(parentItem.id);
+
+ var file = OS.Path.join(await getTempDirectory(), 'empty.pdf');
+ await OS.File.copy(
+ OS.Path.join(getTestDataDirectory().path, 'empty.pdf'),
+ file
+ );
+ file = Zotero.File.pathToFile(file);
+
+ var promise = waitForItemEvent('add');
+
+ itemsView.drop(parentRow, 0, {
+ dropEffect: 'link',
+ effectAllowed: 'link',
+ types: {
+ contains: function (type) {
+ return type == 'application/x-moz-file';
+ }
+ },
+ mozItemCount: 1,
+ mozGetDataAt: function (type, i) {
+ if (type == 'application/x-moz-file' && i == 0) {
+ return file;
+ }
+ }
+ })
+
+ var itemIDs = await promise;
+ var item = Zotero.Items.get(itemIDs[0]);
+ assert.equal(item.parentItemID, parentItem.id);
+ var title = item.getField('title');
+ var path = await item.getFilePathAsync();
+ assert.equal(title, parentTitle + '.pdf');
+ assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
+ });
+
+ it("shouldn't rename a stored child attachment using parent metadata if pref disabled", async function () {
+ Zotero.Prefs.set('renameAttachmentFiles.automatic', false);
+
+ var view = zp.itemsView;
+ var parentTitle = Zotero.Utilities.randomString();
+ var parentItem = await createDataObject('item', { title: parentTitle });
+ await Zotero.Attachments.linkFromURL({
+ url: 'https://example.com',
+ title: 'Example',
+ parentItemID: parentItem.id
+ });
+ var parentRow = view.getRowIndexByID(parentItem.id);
+
+ var originalFileName = 'empty.pdf';
+ var file = getTestDataDirectory();
+ file.append(originalFileName);
+
+ var promise = waitForItemEvent('add');
+
+ itemsView.drop(parentRow, 0, {
+ dropEffect: 'copy',
+ effectAllowed: 'copy',
+ types: {
+ contains: function (type) {
+ return type == 'application/x-moz-file';
+ }
+ },
+ mozItemCount: 1,
+ mozGetDataAt: function (type, i) {
+ if (type == 'application/x-moz-file' && i == 0) {
+ return file;
+ }
+ }
+ })
+
+ var itemIDs = await promise;
+ var item = Zotero.Items.get(itemIDs[0]);
+ assert.equal(item.parentItemID, parentItem.id);
+ var title = item.getField('title');
+ var path = await item.getFilePathAsync();
+ // Should match original filename, not parent title
+ assert.equal(title, originalFileName);
+ assert.equal(OS.Path.basename(path), originalFileName);
+ });
+
+ it("shouldn't rename a stored child attachment using parent metadata if existing file attachments", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
@@ -924,8 +1020,9 @@ describe("Zotero.ItemTreeView", function() {
});
var parentRow = view.getRowIndexByID(parentItem.id);
+ var originalFileName = 'empty.pdf';
var file = getTestDataDirectory();
- file.append('test.png');
+ file.append(originalFileName);
var promise = waitForItemEvent('add');
@@ -950,18 +1047,19 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
- assert.equal(title, 'test.png');
- assert.equal(OS.Path.basename(path), 'test.png');
+ assert.equal(title, originalFileName);
+ assert.equal(OS.Path.basename(path), originalFileName);
});
- it("shouldn't rename a child attachment using parent metadata if drag includes multiple files", async function () {
+ it("shouldn't rename a stored child attachment using parent metadata if drag includes multiple files", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
var parentRow = view.getRowIndexByID(parentItem.id);
+ var originalFileName = 'empty.pdf';
var file = getTestDataDirectory();
- file.append('test.png');
+ file.append(originalFileName);
var promise = waitForItemEvent('add');
@@ -986,8 +1084,8 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
- assert.equal(title, 'test.png');
- assert.equal(OS.Path.basename(path), 'test.png');
+ assert.equal(title, originalFileName);
+ assert.equal(OS.Path.basename(path), originalFileName);
});
});
})
diff --git a/test/tests/recognizePDFTest.js b/test/tests/recognizePDFTest.js
index 092649c249..5bc9d4ae6a 100644
--- a/test/tests/recognizePDFTest.js
+++ b/test/tests/recognizePDFTest.js
@@ -27,31 +27,36 @@ describe("PDF Recognition", function() {
}
});
- it("should recognize a PDF", function* () {
+ it("should recognize a PDF", async function () {
this.timeout(30000);
// Import the PDF
var testdir = getTestDataDirectory();
testdir.append("recognizePDF_test_GS.pdf");
- var item = yield Zotero.Attachments.importFromFile({
+ var attachment = await Zotero.Attachments.importFromFile({
file: testdir
});
// Recognize the PDF
win.ZoteroPane.recognizeSelected();
- var addedIDs = yield waitForItemEvent("add");
- var modifiedIDs = yield waitForItemEvent("modify");
+ var addedIDs = await waitForItemEvent("add");
+ var modifiedIDs = await waitForItemEvent("modify");
assert.lengthOf(addedIDs, 1);
var item = Zotero.Items.get(addedIDs[0]);
assert.equal(item.getField("title"), "Scaling study of an improved fermion action on quenched lattices");
assert.lengthOf(modifiedIDs, 2);
- yield Zotero.Promise.delay(0);
-
+ // Wait for status to show as complete
var progressWindow = getWindows("chrome://zotero/content/recognizePDFDialog.xul")[0];
+ var completeStr = Zotero.getString("recognizePDF.complete.label");
+ while (progressWindow.document.getElementById("label").value != completeStr) {
+ await Zotero.Promise.delay(20);
+ }
+
+ // The file should have been renamed
assert.equal(
- progressWindow.document.getElementById("label").value,
- Zotero.getString("recognizePDF.complete.label")
+ attachment.attachmentFilename,
+ Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
);
});
});
\ No newline at end of file