Bug 1437250 - Bookmark url queries shouldn't return folder shortcuts. r?mak draft
authorMark Banner <standard8@mozilla.com>
Tue, 20 Feb 2018 20:34:09 +0000
changeset 757898 c9608896d54b007945fb36213d529c6c6347aade
parent 757856 dd6caba141428343fb26f3ec23a3ee1844a4b241
push id99872
push userbmo:standard8@mozilla.com
push dateWed, 21 Feb 2018 14:15:03 +0000
reviewersmak
bugs1437250
milestone60.0a1
Bug 1437250 - Bookmark url queries shouldn't return folder shortcuts. r?mak URL queries don't need to return queries not shortcuts, as that isn't what the invoker is expecting. MozReview-Commit-ID: 74LvXqc40Ps
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/queries/test_excludeQueries.js
toolkit/components/places/tests/queries/test_options_inherit.js
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -1599,16 +1599,18 @@ PlacesSQLQueryBuilder::SelectAsURI()
             "null, null, null, b.guid, b.position, b.type, b.fk "
           "FROM moz_bookmarks b "
           "JOIN moz_places h ON b.fk = h.id "
           "WHERE NOT EXISTS "
               "(SELECT id FROM moz_bookmarks "
                 "WHERE id = b.parent AND parent = ") +
                   nsPrintfCString("%" PRId64, history->GetTagsFolder()) +
               NS_LITERAL_CSTRING(") "
+              "AND NOT h.url_hash BETWEEN hash('place', 'prefix_lo') AND "
+                                         "hash('place', 'prefix_hi') "
             "{ADDITIONAL_CONDITIONS}");
       }
       break;
 
     default:
       return NS_ERROR_NOT_IMPLEMENTED;
   }
   return NS_OK;
--- a/toolkit/components/places/tests/queries/test_excludeQueries.js
+++ b/toolkit/components/places/tests/queries/test_excludeQueries.js
@@ -26,20 +26,21 @@ add_task(async function setup() {
     title: "a bookmark"
   });
 
   checkBookmarkObject(bm);
   checkBookmarkObject(fakeQuery);
   checkBookmarkObject(folderShortcut);
 });
 
-add_task(async function test_bookmarks_excludeQueries() {
-  // When excluding queries, we exclude actual queries, but not folder shortcuts.
-  let expectedGuids = [bm.guid, folderShortcut.guid];
-
+add_task(async function test_bookmarks_url_query_implicit_exclusions() {
+  // When we run bookmarks url queries, we implicity filter out queries and
+  // folder shortcuts regardless of excludeQueries. They don't make sense to
+  // include in the results.
+  let expectedGuids = [bm.guid];
   let query = PlacesUtils.history.getNewQuery();
   let options = PlacesUtils.history.getNewQueryOptions();
   options.excludeQueries = true;
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
 
   let root = PlacesUtils.history.executeQuery(query, options).root;
   root.containerOpen = true;
 
@@ -47,16 +48,37 @@ add_task(async function test_bookmarks_e
   for (let i = 0; i < expectedGuids.length; i++) {
     Assert.equal(root.getChild(i).bookmarkGuid, expectedGuids[i],
       "should have got the expected item");
   }
 
   root.containerOpen = false;
 });
 
+
+add_task(async function test_bookmarks_excludeQueries() {
+  // When excluding queries, we exclude actual queries, but not folder shortcuts.
+  let expectedGuids = [bm.guid, folderShortcut.guid];
+  let query = {};
+  let options = {};
+  let queryString = `place:folder=${PlacesUtils.unfiledBookmarksFolderId}&excludeQueries=1`;
+  PlacesUtils.history.queryStringToQueries(queryString, query, {}, options);
+
+  let root = PlacesUtils.history.executeQuery(query.value[0], options.value).root;
+  root.containerOpen = true;
+
+  Assert.equal(root.childCount, expectedGuids.length, "Checking root child count");
+  for (let i = 0; i < expectedGuids.length; i++) {
+    Assert.equal(root.getChild(i).bookmarkGuid, expectedGuids[i],
+      "should have got the expected item");
+  }
+
+  root.containerOpen = false;
+});
+
 add_task(async function test_search_excludesQueries() {
   // Searching implicity removes queries and folder shortcuts even if excludeQueries
   // is not specified.
   let expectedGuids = [bm.guid];
 
   let query = PlacesUtils.history.getNewQuery();
   query.searchTerms = "bookmark";
 
--- a/toolkit/components/places/tests/queries/test_options_inherit.js
+++ b/toolkit/components/places/tests/queries/test_options_inherit.js
@@ -31,17 +31,17 @@ add_task(async function() {
         url: "http://example.com",
       },
       {
         type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
       },
     ]
   });
 
-  await test_query({}, 3, 3, 3);
+  await test_query({}, 3, 3, 2);
   await test_query({ expandQueries: false }, 3, 3, 0);
   await test_query({ excludeItems: true }, 1, 1, 0);
   await test_query({ excludeItems: true, expandQueries: false }, 1, 1, 0);
   await test_query({ excludeItems: true, excludeQueries: true }, 1, 0, 0);
 });
 
 async function test_query(opts, expectedRootCc, expectedFolderCc, expectedQueryCc) {
   let query = PlacesUtils.history.getNewQuery();