diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 4034bb8366..9d61f0f3e0 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2390,9 +2390,8 @@ Zotero.Attachments = new function () { }, {}); const firstCreator = args => common( - // 74492e40 adds \u2068 and \u2069 around names in the `firstCreator` field, which we don't want in the filename - // We might actually want to move this replacement to getValidFileName - item.getField('firstCreator', false, true).replaceAll('\u2068', '').replaceAll('\u2069', ''), args + // Pass unformatted = true to omit bidi isolates + item.getField('firstCreator', true, true), args ); const vars = { ...fields, ...creatorFields, firstCreator, itemType, year }; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index c37de5900b..ecbdd5612f 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -216,12 +216,12 @@ Zotero.Item.prototype._setParentKey = function() { // Public Zotero.Item methods // ////////////////////////////////////////////////////////////////////////////// -/* +/** * Retrieves an itemData field value * * @param {String|Integer} field fieldID or fieldName - * @param {Boolean} [unformatted] Skip any special processing of DB value - * (e.g. multipart date field) + * @param {Boolean} [unformatted] Skip formatting of multipart date fields and + * omit bidi control characters * @param {Boolean} includeBaseMapped If true and field is a base field, returns * value of type-specific field instead * (e.g. 'label' for 'publisher' in 'audioRecording') @@ -238,11 +238,17 @@ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped) if (field === 'firstCreator' && !this._id) { // Hack to get a firstCreator for an unsaved item let creatorsData = this.getCreators(true); - return Zotero.Items.getFirstCreatorFromData(this.itemTypeID, creatorsData); + return Zotero.Items.getFirstCreatorFromData(this.itemTypeID, creatorsData, + { omitBidiIsolates: !!unformatted }); } else if (field === 'id' || this.ObjectsClass.isPrimaryField(field)) { var privField = '_' + field; - //Zotero.debug('Returning ' + (this[privField] ? this[privField] : '') + ' (typeof ' + typeof this[privField] + ')'); - return this[privField]; + let value = this[privField]; + // Bidi isolates + if (unformatted && field === 'firstCreator') { + value = value.replace(/[\u2068\u2069]/g, ''); + } + //Zotero.debug('Returning ' + (value ? value : '') + ' (typeof ' + typeof value + ')'); + return value; } else if (field == 'year') { return this.getField('date', true, true).substr(0,4); } diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index e3c25bada5..a72cc81375 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -1749,9 +1749,17 @@ Zotero.Items = function() { * * @param {Integer} itemTypeID * @param {Object} creatorData + * @param {Object} [options] + * @param {Boolean} [options.omitBidiIsolates] * @return {String} */ - this.getFirstCreatorFromData = function (itemTypeID, creatorsData) { + this.getFirstCreatorFromData = function (itemTypeID, creatorsData, options) { + if (!options) { + options = { + omitBidiIsolates: false + }; + } + if (creatorsData.length === 0) { return ""; } @@ -1773,9 +1781,12 @@ Zotero.Items = function() { if (matches.length === 2) { let a = matches[0]; let b = matches[1]; - // \u2068 FIRST STRONG ISOLATE: Isolates the directionality of characters that follow - // \u2069 POP DIRECTIONAL ISOLATE: Pops the above isolation - return Zotero.getString('general.andJoiner', [`\u2068${a.lastName}\u2069`, `\u2068${b.lastName}\u2069`]); + let args = options.omitBidiIsolates + ? [a.lastName, b.lastName] + // \u2068 FIRST STRONG ISOLATE: Isolates the directionality of characters that follow + // \u2069 POP DIRECTIONAL ISOLATE: Pops the above isolation + : [`\u2068${a.lastName}\u2069`, `\u2068${b.lastName}\u2069`]; + return Zotero.getString('general.andJoiner', args); } if (matches.length >= 3) { return matches[0].lastName + " " + Zotero.getString('general.etAl'); diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js index 433ac3d89d..7cc97bcd62 100644 --- a/chrome/content/zotero/xpcom/file.js +++ b/chrome/content/zotero/xpcom/file.js @@ -1267,6 +1267,8 @@ Zotero.File = new function(){ // Normalize to NFC fileName = fileName.normalize(); } + // Replace bidi isolation control characters + fileName = fileName.replace(/[\u2068\u2069]/g, ''); // Don't allow hidden files fileName = fileName.replace(/^\./, ''); // Don't allow blank or illegal filenames diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index 639a6c9d63..d33c6b1aef 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -181,7 +181,10 @@ class ReaderInstance { let attachment = await parentItem.getBestAttachment(); if (attachment && attachment.id === this._itemID) { let parts = []; - let creator = parentItem.getField('firstCreator'); + // Windows displays bidi control characters as placeholders in window titles, so strip them + // See https://github.com/mozilla-services/screenshots/issues/4863 + let unformatted = Zotero.isWin; + let creator = parentItem.getField('firstCreator', unformatted); let year = parentItem.getField('year'); let title = parentItem.getDisplayTitle(); // If creator is missing fall back to titleCreatorYear @@ -198,11 +201,6 @@ class ReaderInstance { readerTitle = parts.filter(x => x).join(' - '); } } - if (Zotero.isWin) { - // Windows displays bidi control characters as placeholders in window titles, so strip them - // See https://github.com/mozilla-services/screenshots/issues/4863 - readerTitle = readerTitle.replace(/[\u2068\u2069]/g, ''); - } this._title = readerTitle; this._setTitleValue(readerTitle); } diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index df4a45d4a3..19e38d22a9 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -1584,6 +1584,13 @@ describe("Zotero.Attachments", function() { 'foo{{ firstCreator }}-{{ title truncate="10" }}-{{ year truncate="2" suffix="00" }}' ); }); + + it("should strip bidi isolates from firstCreator", async function () { + var item = createUnsavedDataObject('item', + { creators: [{ name: 'Foo', creatorType: 'author' }, { name: 'Bar', creatorType: 'author' }] }); + var str = Zotero.Attachments.getFileBaseNameFromItem(item); + assert.equal(str, Zotero.getString('general.andJoiner', ['Foo', 'Bar']) + ' - '); + }); }); describe("#getBaseDirectoryRelativePath()", function () { diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index a04f9c07e3..5eedebeab9 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -35,6 +35,56 @@ describe("Zotero.Item", function () { ]); assert.equal(item.getField('firstCreator'), "B"); }); + + it("should return a multi-author firstCreator for an unsaved item", async function () { + var item = createUnsavedDataObject('item'); + item.setCreators([ + { + firstName: "A", + lastName: "B", + creatorType: "author" + }, + { + firstName: "C", + lastName: "D", + creatorType: "author" + } + ]); + assert.equal( + item.getField('firstCreator'), + Zotero.getString('general.andJoiner', ['\u2068B\u2069', '\u2068D\u2069']) + ); + }); + + it("should strip bidi isolates from firstCreator when unformatted = true", async function () { + var item = createUnsavedDataObject('item'); + item.setCreators([ + { + firstName: "A", + lastName: "B", + creatorType: "author" + }, + { + firstName: "C", + lastName: "D", + creatorType: "author" + } + ]); + + // Test unsaved - uses getFirstCreatorFromData()'s omitBidiIsolates option + assert.equal( + item.getField('firstCreator', /* unformatted */ true), + Zotero.getString('general.andJoiner', ['B', 'D']) + ); + + await item.saveTx(); + + // Test saved - implemented in getField() + assert.equal( + item.getField('firstCreator', /* unformatted */ true), + Zotero.getString('general.andJoiner', ['B', 'D']) + ); + }); }); describe("#setField", function () {