diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 7504bc3d2048..5841090b8fee 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -267,8 +267,18 @@ BookmarksEngine.prototype = { let id = this._store.idForGUID(guid); switch (Svc.Bookmark.getItemType(id)) { case Svc.Bookmark.TYPE_BOOKMARK: - key = "b" + Svc.Bookmark.getBookmarkURI(id).spec + ":" + - Svc.Bookmark.getItemTitle(id); + + // Smart bookmarks map to their annotation value. + let queryId; + try { + queryId = Utils.anno(id, SMART_BOOKMARKS_ANNO); + } catch(ex) {} + + if (queryId) + key = "q" + queryId; + else + key = "b" + Svc.Bookmark.getBookmarkURI(id).spec + ":" + + Svc.Bookmark.getItemTitle(id); break; case Svc.Bookmark.TYPE_FOLDER: key = "f" + Svc.Bookmark.getItemTitle(id); @@ -302,9 +312,19 @@ BookmarksEngine.prototype = { return this._lazyMap = function(item) { // Figure out if we have something to key with let key; + let altKey; switch (item.type) { - case "bookmark": case "query": + // Prior to Bug 610501, records didn't carry their Smart Bookmark + // anno, so we won't be able to dupe them correctly. This altKey + // hack should get them to dupe correctly. + if (item.queryId) { + key = "q" + item.queryId; + 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; break; @@ -322,9 +342,29 @@ BookmarksEngine.prototype = { // Give the guid if we have the matching pair this._log.trace("Finding mapping: " + item.parentName + ", " + key); let parent = lazyMap[item.parentName]; - let dupe = parent && parent[key]; - this._log.trace("Mapped dupe: " + dupe); - return dupe; + + if (!parent) { + this._log.trace("No parent => no dupe."); + return undefined; + } + + let dupe = parent[key]; + + if (dupe) { + this._log.trace("Mapped dupe: " + dupe); + return dupe; + } + + if (altKey) { + dupe = parent[altKey]; + if (dupe) { + this._log.trace("Mapped dupe using altKey " + altKey + ": " + dupe); + return dupe; + } + } + + this._log.trace("No dupe found for key " + key + "/" + altKey + "."); + return undefined; }; }); diff --git a/services/sync/tests/unit/test_bookmark_smart_bookmarks.js b/services/sync/tests/unit/test_bookmark_smart_bookmarks.js index 370b15fbde40..25f0b6bbd52e 100644 --- a/services/sync/tests/unit/test_bookmark_smart_bookmarks.js +++ b/services/sync/tests/unit/test_bookmark_smart_bookmarks.js @@ -178,6 +178,68 @@ function test_annotation_uploaded() { } } +function test_smart_bookmarks_duped() { + let parent = PlacesUtils.toolbarFolderId; + let uri = + Utils.makeURI("place:redirectsMode=" + + Ci.nsINavHistoryQueryOptions.REDIRECTS_MODE_TARGET + + "&sort=" + + Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING + + "&maxResults=10"); + let title = "Most Visited"; + let mostVisitedID = newSmartBookmark(parent, uri, -1, title, "MostVisited"); + let mostVisitedGUID = store.GUIDForId(mostVisitedID); + + let record = store.createRecord(mostVisitedGUID); + + _("Prepare sync."); + Svc.Prefs.set("username", "foo"); + Service.serverURL = "http://localhost:8080/"; + Service.clusterURL = "http://localhost:8080/"; + + let collection = new ServerCollection({}, true); + let global = new ServerWBO('global', + {engines: {bookmarks: {version: engine.version, + syncID: engine.syncID}}}); + let server = httpd_setup({ + "/1.0/foo/storage/meta/global": global.handler(), + "/1.0/foo/storage/bookmarks": collection.handler() + }); + + try { + engine._syncStartup(); + + _("Verify that lazyMap uses the anno, discovering a dupe regardless of URI."); + do_check_eq(mostVisitedGUID, engine._lazyMap(record)); + + record.bmkUri = "http://foo/"; + do_check_eq(mostVisitedGUID, engine._lazyMap(record)); + do_check_neq(Svc.Bookmark.getBookmarkURI(mostVisitedID).spec, record.bmkUri); + + _("Verify that different annos don't dupe."); + let other = new BookmarkQuery("bookmarks", "abcdefabcdef"); + other.queryId = "LeastVisited"; + other.parentName = "Bookmarks Toolbar"; + other.bmkUri = "place:foo"; + other.title = ""; + do_check_eq(undefined, engine._findDupe(other)); + + _("Handle records without a queryId entry."); + record.bmkUri = uri; + delete record.queryId; + do_check_eq(mostVisitedGUID, engine._lazyMap(record)); + + engine._syncFinish(); + + } finally { + // Clean up. + store.wipe(); + server.stop(do_test_finished); + Svc.Prefs.resetBranch(""); + Records.clearCache(); + } +} + function run_test() { initTestLogging("Trace"); Log4Moz.repository.getLogger("Engine.Bookmarks").level = Log4Moz.Level.Trace; @@ -185,4 +247,5 @@ function run_test() { CollectionKeys.generateNewKeys(); test_annotation_uploaded(); + test_smart_bookmarks_duped(); }