diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 2ddd4cf5297..7a4ffbc8c3c 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -808,6 +808,11 @@ BookmarksStore.prototype = { this.removeById(itemId, record.id); }, + _taggableTypes: ["bookmark", "microsummary", "query"], + isTaggable: function isTaggable(recordType) { + return this._taggableTypes.indexOf(recordType) != -1; + }, + update: function BStore_update(record) { let itemId = this.idForGUID(record.id); @@ -852,9 +857,12 @@ BookmarksStore.prototype = { PlacesUtils.bookmarks.changeBookmarkURI(itemId, Utils.makeURI(val)); break; case "tags": - if (Array.isArray(val) && - (remoteRecordType in ["bookmark", "microsummary", "query"])) { - this._tagID(itemId, val); + if (Array.isArray(val)) { + if (this.isTaggable(remoteRecordType)) { + this._tagID(itemId, val); + } else { + this._log.debug("Remote record type is invalid for tags: " + remoteRecordType); + } } break; case "keyword": @@ -1230,9 +1238,12 @@ BookmarksStore.prototype = { } try { - let u = PlacesUtils.bookmarks.getBookmarkURI(itemId); - _tagURI(u, tags); + let u = PlacesUtils.bookmarks.getBookmarkURI(itemID); + this._tagURI(u, tags); } catch (e) { + this._log.warn("Got exception fetching URI for " + itemID + ": not tagging. " + + Utils.exceptionStr(e)); + // I guess it doesn't have a URI. Don't try to tag it. return; } diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index 1eae5643209..51a9e8bf8ab 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -418,6 +418,22 @@ add_test(function test_bookmark_guidMap_fail() { server.stop(run_next_test); }); +add_test(function test_bookmark_is_taggable() { + let engine = new BookmarksEngine(); + let store = engine._store; + + do_check_true(store.isTaggable("bookmark")); + do_check_true(store.isTaggable("microsummary")); + do_check_true(store.isTaggable("query")); + do_check_false(store.isTaggable("folder")); + do_check_false(store.isTaggable("livemark")); + do_check_false(store.isTaggable(null)); + do_check_false(store.isTaggable(undefined)); + do_check_false(store.isTaggable("")); + + run_next_test(); +}); + add_test(function test_bookmark_tag_but_no_uri() { _("Ensure that a bookmark record with tags, but no URI, doesn't throw an exception.");