From 87901c1c210e79a6f16f83554f82adfbdd2f80bb Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 31 May 2026 17:03:45 +0200 Subject: [PATCH] Fix error when removing a tag stored in a non-normalized form Zotero.Tags.init() keyed the id cache by the raw tag name from the database, but getID() and Items._loadTags() both normalize names via cleanData() (trim + NFC). A tag stored in a non-normalized form (e.g., non-NFC or with surrounding whitespace) could therefore never be matched by getID(), which returned false. When such a tag was removed from an item, that false was bound as the tagID parameter in the DELETE FROM itemTags query, throwing "Invalid boolean parameter 1 'false'" and aborting the save. Since this fired while applying changes during sync, it persistently blocked sync. To fix, normalize names in init() to match getID()/_loadTags(). https://forums.zotero.org/discussion/131901/error-report-seems-to-be-preventing-sync --- chrome/content/zotero/xpcom/data/tags.js | 7 ++- test/tests/tagsTest.js | 74 +++++++++++++++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js index be8d12914c..8003462dd5 100644 --- a/chrome/content/zotero/xpcom/data/tags.js +++ b/chrome/content/zotero/xpcom/data/tags.js @@ -48,10 +48,13 @@ Zotero.Tags = new function () { { onRow: function (row) { var tagID = row.getResultByIndex(0); - var name = row.getResultByIndex(1); + // Normalize the name to match getID() and Items._loadTags(), which clean + // tags on lookup and load -- otherwise a tag stored in a non-normalized + // form (e.g., non-NFC or with surrounding whitespace) can never be matched + var name = this.cleanData({ tag: row.getResultByIndex(1) }).tag; _tagsByID.set(tagID, name); _idsByTag.set(name, tagID); - } + }.bind(this) } ); _initialized = true; diff --git a/test/tests/tagsTest.js b/test/tests/tagsTest.js index 4da4806a76..838f31a225 100644 --- a/test/tests/tagsTest.js +++ b/test/tests/tagsTest.js @@ -10,8 +10,80 @@ describe("Zotero.Tags", function () { assert.typeOf(Zotero.Tags.getID(tagName), "number"); }) + + it("should find a tag stored in a non-normalized form", async function () { + // Random ASCII prefix + a combining acute accent, so the name is unique + // per run but stored in a non-normalized (NFD) form + var nfd = Zotero.Utilities.randomString() + 'e\u0301'; + var nfc = nfd.normalize(); + assert.notEqual(nfd, nfc); + + var item = await createDataObject('item'); + + // Write the tag straight to the DB in non-normalized form, bypassing the + // normalization that Zotero.Tags.create() performs, and attach it to the item + var tagID = Zotero.ID.get('tags'); + await Zotero.DB.executeTransaction(async function () { + await Zotero.DB.queryAsync( + "INSERT INTO tags (tagID, name) VALUES (?, ?)", [tagID, nfd] + ); + await Zotero.DB.queryAsync( + "INSERT INTO itemTags (itemID, tagID, type) VALUES (?, ?, 0)", + [item.id, tagID] + ); + }); + + // Rebuild the tag cache from the DB so it picks up the non-normalized name + await Zotero.Tags.init(); + + // getID() normalizes its lookup, so it should still match the stored tag + assert.equal(Zotero.Tags.getID(nfc), tagID); + }) }) - + + describe("non-normalized tags", function () { + // Tags loaded onto an item are normalized (NFC, trimmed), so removing a tag that's + // stored in a non-normalized form requires getID() to resolve the normalized name. + // Previously the id cache was keyed by the raw DB name, so getID() returned false, + // and the false was bound as the tagID parameter -- "Invalid boolean parameter 1 + // 'false' [QUERY: DELETE FROM itemTags WHERE itemID=? AND tagID=? AND type=?]". + it("should remove a tag stored in a non-normalized form without throwing", async function () { + // Random ASCII prefix + a combining acute accent, so the name is unique + // per run but stored in a non-normalized (NFD) form + var nfd = Zotero.Utilities.randomString() + 'e\u0301'; + var nfc = nfd.normalize(); + assert.notEqual(nfd, nfc); + + var item = await createDataObject('item'); + + var tagID = Zotero.ID.get('tags'); + await Zotero.DB.executeTransaction(async function () { + await Zotero.DB.queryAsync( + "INSERT INTO tags (tagID, name) VALUES (?, ?)", [tagID, nfd] + ); + await Zotero.DB.queryAsync( + "INSERT INTO itemTags (itemID, tagID, type) VALUES (?, ?, 0)", + [item.id, tagID] + ); + }); + + await Zotero.Tags.init(); + await item.loadDataType('tags', true); + + // The loaded tag is normalized + assert.sameDeepMembers(item.getTags(), [{ tag: nfc }]); + + item.removeTag(nfc); + await item.saveTx(); + + assert.lengthOf(item.getTags(), 0); + var count = await Zotero.DB.valueQueryAsync( + "SELECT COUNT(*) FROM itemTags WHERE itemID=?", item.id + ); + assert.equal(count, 0); + }) + }) + describe("#getName()", function () { it("should return tag id", async function () { var tagName = Zotero.Utilities.randomString();