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

@@ -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;

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();