diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 2f66bf3592be..f9f67a474b90 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -347,12 +347,12 @@ BookmarksEngine.prototype = { if (query && query.value) { key = "q" + query.value; } else { - key = "b" + node.uri + ":" + (node.title || ""); + key = "b" + node.uri + ":" + node.title; } break; case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER: // Folder - key = "f" + (node.title || ""); + key = "f" + node.title; break; case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR: // Separator @@ -363,7 +363,7 @@ BookmarksEngine.prototype = { continue; } - let parentName = parent.title || ""; + let parentName = parent.title; if (guidMap[parentName] == null) guidMap[parentName] = {}; @@ -391,17 +391,17 @@ BookmarksEngine.prototype = { // hack should get them to dupe correctly. if (item.queryId) { key = "q" + item.queryId; - altKey = "b" + item.bmkUri + ":" + (item.title || ""); + altKey = "b" + item.bmkUri + ":" + item.title; break; } // No queryID? Fall through to the regular bookmark case. case "bookmark": case "microsummary": - key = "b" + item.bmkUri + ":" + (item.title || ""); + key = "b" + item.bmkUri + ":" + item.title; break; case "folder": case "livemark": - key = "f" + (item.title || ""); + key = "f" + item.title; break; case "separator": key = "s" + item.pos; @@ -415,9 +415,8 @@ BookmarksEngine.prototype = { let guidMap = this._guidMap; // Give the GUID if we have the matching pair. - let parentName = item.parentName || ""; - this._log.trace("Finding mapping: " + parentName + ", " + key); - let parent = guidMap[parentName]; + this._log.trace("Finding mapping: " + item.parentName + ", " + key); + let parent = guidMap[item.parentName]; if (!parent) { this._log.trace("No parent => no dupe."); diff --git a/toolkit/components/places/PlacesSyncUtils.jsm b/toolkit/components/places/PlacesSyncUtils.jsm index 1a28bc30bc68..0bc387d9e3a3 100644 --- a/toolkit/components/places/PlacesSyncUtils.jsm +++ b/toolkit/components/places/PlacesSyncUtils.jsm @@ -279,9 +279,7 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({ } // Convert the Places bookmark object to a Sync bookmark and add - // kind-specific properties. Titles are required for bookmarks, - // folders, and livemarks; optional for queries, and omitted for - // separators. + // kind-specific properties. let kind = yield getKindForItem(bookmarkItem); let item; switch (kind) { @@ -311,11 +309,12 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({ throw new Error(`Unknown bookmark kind: ${kind}`); } - // Sync uses the parent title for de-duping. All Sync bookmark objects - // except the Places root should have this property. + // Sync uses the parent title for de-duping. if (bookmarkItem.parentGuid) { let parent = yield PlacesUtils.bookmarks.fetch(bookmarkItem.parentGuid); - item.parentTitle = parent.title || ""; + if ("title" in parent) { + item.parentTitle = parent.title; + } } return item; @@ -1041,10 +1040,6 @@ function syncBookmarkToPlacesBookmark(info) { var fetchBookmarkItem = Task.async(function* (bookmarkItem) { let item = yield placesBookmarkToSyncBookmark(bookmarkItem); - if (!item.title) { - item.title = ""; - } - item.tags = PlacesUtils.tagging.getTagsForURI( PlacesUtils.toURI(bookmarkItem.url), {}); @@ -1072,10 +1067,6 @@ var fetchBookmarkItem = Task.async(function* (bookmarkItem) { var fetchFolderItem = Task.async(function* (bookmarkItem) { let item = yield placesBookmarkToSyncBookmark(bookmarkItem); - if (!item.title) { - item.title = ""; - } - let description = yield getAnno(bookmarkItem.guid, BookmarkSyncUtils.DESCRIPTION_ANNO); if (description) { @@ -1096,10 +1087,6 @@ var fetchFolderItem = Task.async(function* (bookmarkItem) { var fetchLivemarkItem = Task.async(function* (bookmarkItem) { let item = yield placesBookmarkToSyncBookmark(bookmarkItem); - if (!item.title) { - item.title = ""; - } - let description = yield getAnno(bookmarkItem.guid, BookmarkSyncUtils.DESCRIPTION_ANNO); if (description) { diff --git a/toolkit/components/places/tests/unit/test_sync_utils.js b/toolkit/components/places/tests/unit/test_sync_utils.js index f8c7e6b58fc3..f89c3579d9fc 100644 --- a/toolkit/components/places/tests/unit/test_sync_utils.js +++ b/toolkit/components/places/tests/unit/test_sync_utils.js @@ -1058,15 +1058,14 @@ add_task(function* test_fetch() { description: "Folder description", childSyncIds: [folderBmk.syncId, folderSep.syncId], parentTitle: "Bookmarks Menu", - title: "", - }, "Should include description, children, title, and parent title in folder"); + }, "Should include description, children, and parent title in folder"); } do_print("Fetch bookmark with description, sidebar anno, and tags"); { let item = yield PlacesSyncUtils.bookmarks.fetch(bmk.syncId); - deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId", - "url", "tags", "description", "loadInSidebar", "parentTitle", "title"].sort(), + deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId", "url", + "tags", "description", "loadInSidebar", "parentTitle"].sort(), "Should include bookmark-specific properties"); equal(item.syncId, bmk.syncId, "Sync ID should match"); equal(item.url.href, "https://example.com/", "Should return URL"); @@ -1075,20 +1074,17 @@ add_task(function* test_fetch() { equal(item.description, "Bookmark description", "Should return bookmark description"); strictEqual(item.loadInSidebar, true, "Should return sidebar anno"); equal(item.parentTitle, "Bookmarks Menu", "Should return parent title"); - strictEqual(item.title, "", "Should return empty title"); } do_print("Fetch bookmark with keyword; without parent title or annos"); { let item = yield PlacesSyncUtils.bookmarks.fetch(folderBmk.syncId); deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId", - "url", "keyword", "tags", "loadInSidebar", "parentTitle", "title"].sort(), + "url", "keyword", "tags", "loadInSidebar"].sort(), "Should omit blank bookmark-specific properties"); strictEqual(item.loadInSidebar, false, "Should not load bookmark in sidebar"); deepEqual(item.tags, [], "Tags should be empty"); equal(item.keyword, "kw", "Should return keyword"); - strictEqual(item.parentTitle, "", "Should include parent title even if empty"); - strictEqual(item.title, "", "Should include bookmark title even if empty"); } do_print("Fetch separator"); @@ -1136,12 +1132,11 @@ add_task(function* test_fetch_livemark() { do_print("Fetch livemark"); let item = yield PlacesSyncUtils.bookmarks.fetch(livemark.guid); deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId", - "description", "feed", "site", "parentTitle", "title"].sort(), + "description", "feed", "site", "parentTitle"].sort(), "Should include livemark-specific properties"); equal(item.description, "Livemark description", "Should return description"); equal(item.feed.href, site + "/feed/1", "Should return feed URL"); equal(item.site.href, site + "/", "Should return site URL"); - strictEqual(item.title, "", "Should include livemark title even if empty"); } finally { yield stopServer(); }