Bug 1358127 - Fix bookmarks.search so it doesn't return the contents of tag folders, r=mak
authorBob Silverberg <bsilverberg@mozilla.com>
Mon, 24 Apr 2017 09:04:59 -0400
changeset 355195 81a7160c5cbccb65e9b95486d2b5ae9074aa3bcc
parent 355194 ff2157da967306962fd12b33a49ae7c7d791969c
child 355196 ce13c8b07ca7cae81d32f802b97c14fb4cad8b34
push id41612
push userbsilverberg@mozilla.com
push dateThu, 27 Apr 2017 11:45:56 +0000
treeherderautoland@81a7160c5cbc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1358127
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1358127 - Fix bookmarks.search so it doesn't return the contents of tag folders, r=mak Also fix bookmarks.search so it doesn't return separators. MozReview-Commit-ID: 18tkepk72f8
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_search.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -1305,19 +1305,24 @@ function insertBookmarkTree(items, sourc
 
     return items;
   }));
 }
 
 // Query implementation.
 
 function queryBookmarks(info) {
-  let queryParams = {tags_folder: PlacesUtils.tagsFolderId};
-  // we're searching for bookmarks, so exclude tags
-  let queryString = "WHERE p.parent <> :tags_folder";
+  let queryParams = {
+    tags_folder: PlacesUtils.tagsFolderId,
+    type: Bookmarks.TYPE_SEPARATOR,
+  };
+  // We're searching for bookmarks, so exclude tags and separators.
+  let queryString = "WHERE b.type <> :type";
+  queryString += " AND b.parent <> :tags_folder";
+  queryString += " AND p.parent <> :tags_folder";
 
   if (info.title) {
     queryString += " AND b.title = :title";
     queryParams.title = info.title;
   }
 
   if (info.url) {
     queryString += " AND h.url_hash = hash(:url) AND h.url = :url";
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_search.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_search.js
@@ -216,8 +216,44 @@ add_task(function* search_folder() {
   Assert.equal(results.length, 1);
   checkBookmarkObject(results[0]);
   Assert.deepEqual(bm, results[0]);
   Assert.equal(folder.guid, results[0].parentGuid);
 
   yield PlacesUtils.bookmarks.eraseEverything();
 });
 
+add_task(function* search_excludes_separators() {
+  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 url: "http://example.com/",
+                                                 title: "a bookmark" });
+  let bm2 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 type: PlacesUtils.bookmarks.TYPE_SEPARATOR });
+
+  checkBookmarkObject(bm1);
+  checkBookmarkObject(bm2);
+
+  let results = yield PlacesUtils.bookmarks.search({});
+  Assert.ok(results.findIndex(bookmark => { return bookmark.guid == bm1.guid }) > -1,
+            "The bookmark was found in the results.");
+  Assert.equal(-1, results.findIndex(bookmark => { return bookmark.guid == bm2.guid }),
+            "The separator was excluded from the results.");
+
+  yield PlacesUtils.bookmarks.eraseEverything();
+});
+
+add_task(function* search_excludes_tags() {
+  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 url: "http://example.com/",
+                                                 title: "a bookmark" });
+  checkBookmarkObject(bm1);
+  PlacesUtils.tagging.tagURI(uri(bm1.url.href), ["Test Tag"]);
+
+  let results = yield PlacesUtils.bookmarks.search("example.com");
+  // If tags are not being excluded, this would return two results, one representing the tag.
+  Assert.equal(1, results.length, "A single object was returned from search.");
+  Assert.deepEqual(bm1, results[0], "The bookmark was returned.");
+
+  results = yield PlacesUtils.bookmarks.search("Test Tag");
+  Assert.equal(0, results.length, "The tag folder was not returned.");
+
+  yield PlacesUtils.bookmarks.eraseEverything();
+});