Bug 1444262 - ensure that a bookmark that later becomes a query doesn't break the mirror. r?kitcambridge draft
authorMark Hammond <mhammond@skippinet.com.au>
Fri, 09 Mar 2018 12:09:47 +1100
changeset 765098 702c26c4267f5da52008b9305306b58785fd4121
parent 765097 2bd623f5c1484f06de63b6d04e9801afa08c19e7
push id101972
push userbmo:markh@mozilla.com
push dateFri, 09 Mar 2018 01:09:59 +0000
reviewerskitcambridge
bugs1444262
milestone60.0a1
Bug 1444262 - ensure that a bookmark that later becomes a query doesn't break the mirror. r?kitcambridge MozReview-Commit-ID: 5Ni8KFR7krr
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/tests/sync/test_bookmark_kinds.js
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -505,18 +505,18 @@ class SyncedBookmarksMirror {
       MirrorLog.warn("Ignoring bookmark with invalid ID", record.id);
       this.recordTelemetryEvent("mirror", "ignore", "bookmark",
                                 { why: "id" });
       return;
     }
 
     let url = validateURL(record.bmkUri);
     if (!url) {
-      MirrorLog.trace("Ignoring bookmark ${guid} with invalid URL ${url}",
-                      { guid, url: record.bmkUri });
+      MirrorLog.warn("Ignoring bookmark ${guid} with invalid URL ${url}",
+                     { guid, url: record.bmkUri });
       this.recordTelemetryEvent("mirror", "ignore", "bookmark",
                                 { why: "url" });
       return;
     }
 
     await this.maybeStoreRemoteURL(url);
 
     let serverModified = determineServerModified(record);
@@ -562,18 +562,18 @@ class SyncedBookmarksMirror {
       MirrorLog.warn("Ignoring query with invalid ID", record.id);
       this.recordTelemetryEvent("mirror", "ignore", "query",
                                 { why: "id" });
       return;
     }
 
     let url = validateURL(record.bmkUri);
     if (!url) {
-      MirrorLog.trace("Ignoring query ${guid} with invalid URL ${url}",
-                      { guid, url: record.bmkUri });
+      MirrorLog.warn("Ignoring query ${guid} with invalid URL ${url}",
+                     { guid, url: record.bmkUri });
       this.recordTelemetryEvent("mirror", "ignore", "query",
                                 { why: "url" });
       return;
     }
 
     await this.maybeStoreRemoteURL(url);
 
     let serverModified = determineServerModified(record);
@@ -657,18 +657,18 @@ class SyncedBookmarksMirror {
       MirrorLog.warn("Ignoring livemark with invalid ID", record.id);
       this.recordTelemetryEvent("mirror", "ignore", "livemark",
                                 { why: "id" });
       return;
     }
 
     let feedURL = validateURL(record.feedUri);
     if (!feedURL) {
-      MirrorLog.trace("Ignoring livemark ${guid} with invalid feed URL ${url}",
-                      { guid, url: record.feedUri });
+      MirrorLog.warn("Ignoring livemark ${guid} with invalid feed URL ${url}",
+                     { guid, url: record.feedUri });
       this.recordTelemetryEvent("mirror", "ignore", "livemark",
                                 { why: "feed" });
       return;
     }
 
     let serverModified = determineServerModified(record);
     let dateAdded = determineDateAdded(record);
     let title = validateTitle(record.title);
--- a/toolkit/components/places/tests/sync/test_bookmark_kinds.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_kinds.js
@@ -352,8 +352,57 @@ add_task(async function test_mismatched_
     updated: [],
     deleted: [],
   }, "Should not reupload merged livemark");
 
   await buf.finalize();
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
+
+add_task(async function test_mismatched_bookmark_types() {
+  try {
+    let buf = await openMirror("partial_queries");
+
+    await PlacesUtils.bookmarks.insertTree({
+      guid: PlacesUtils.bookmarks.menuGuid,
+      children: [
+        {
+          guid: "bookmarkAAAA",
+          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+          title: "not yet a query",
+          url: "about:blank",
+        },
+      ],
+    });
+
+    let changes = await buf.apply();
+    // We should have an outgoing record for our bookmark with type=bookmark.
+    Assert.equal(changes.bookmarkAAAA.cleartext.type, "bookmark");
+
+    // Now pretend that same record is already on the server.
+    await buf.store([{
+      id: "menu",
+      type: "folder",
+      children: ["bookmarkAAAA"],
+    }, {
+      id: "bookmarkAAAA",
+      parentId: PlacesSyncUtils.bookmarks.guidToRecordId(PlacesUtils.bookmarks.menuGuid),
+      type: "bookmark",
+      title: "not yet a query",
+      bmkUri: "about:blank",
+    }], { needsMerge: false });
+    await PlacesTestUtils.markBookmarksAsSynced();
+
+    // change the url to be a "real" query.
+    await PlacesUtils.bookmarks.update({
+      guid: "bookmarkAAAA",
+      url: "place:type=6&sort=14&maxResults=10",
+    });
+
+    changes = await buf.apply();
+    // We should have an outgoing record for our bookmark with type=query.
+    Assert.equal(changes.bookmarkAAAA.cleartext.type, "query");
+  } finally {
+    await PlacesUtils.bookmarks.eraseEverything();
+    await PlacesSyncUtils.bookmarks.reset();
+  }
+});