Bug 1380835 - Don't check queries with `excludeQueries` for cycles in the bookmark validator. r=tcsc draft
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 13 Jul 2017 19:52:57 -0700
changeset 609026 64d98ffc92954ade0da89d9e2704b62ae4559e97
parent 608658 67cd1ee26f2661fa5efe3d952485ab3c89af4271
child 637482 3ce37c137be03e4c242d2c3161bd172d4e39be87
push id68481
push userbmo:kit@mozilla.com
push dateFri, 14 Jul 2017 16:19:33 +0000
reviewerstcsc
bugs1380835
milestone56.0a1
Bug 1380835 - Don't check queries with `excludeQueries` for cycles in the bookmark validator. r=tcsc MozReview-Commit-ID: DsPzrCxWWMG
services/sync/modules/bookmark_validator.js
services/sync/tests/unit/test_bookmark_validator.js
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -249,16 +249,23 @@ class BookmarkValidator {
   }
 
   _followQueries(recordsByQueryId) {
     for (let entry of recordsByQueryId.values()) {
       if (entry.type !== "query" && (!entry.bmkUri || !entry.bmkUri.startsWith(QUERY_PROTOCOL))) {
         continue;
       }
       let params = new URLSearchParams(entry.bmkUri.slice(QUERY_PROTOCOL.length));
+      // Queries with `excludeQueries` won't form cycles because they'll
+      // exclude all queries, including themselves, from the result set.
+      let excludeQueries = params.get("excludeQueries");
+      if (excludeQueries === "1" || excludeQueries === "true") {
+        // `nsNavHistoryQuery::ParseQueryBooleanString` allows `1` and `true`.
+        continue;
+      }
       entry.concreteItems = [];
       let queryIds = params.getAll("folder");
       for (let queryId of queryIds) {
         let concreteItem = recordsByQueryId.get(queryId);
         if (concreteItem) {
           entry.concreteItems.push(concreteItem);
         }
       }
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -337,16 +337,24 @@ add_task(async function test_cswc_client
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.menuGuid,
     children: [{
       // A query for the menu, referenced by its local ID instead of
       // `BOOKMARKS_MENU`. This should be reported as a cycle.
       guid: "dddddddddddd",
       url: `place:folder=${PlacesUtils.bookmarksMenuFolderId}`,
       title: "Bookmarks Menu",
+    }, {
+      // A query that references the menu, but excludes itself, so it can't
+      // form a cycle.
+      guid: "iiiiiiiiiiii",
+      url: `place:folder=BOOKMARKS_MENU&folder=UNFILED_BOOKMARKS&` +
+           `folder=TOOLBAR&queryType=1&sort=12&maxResults=10&` +
+           `excludeQueries=1`,
+      title: "Recently Bookmarked",
     }],
   });
 
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.toolbarGuid,
     children: [{
       guid: "eeeeeeeeeeee",
       type: PlacesUtils.bookmarks.TYPE_FOLDER,