Bug 1358127 - Fix bookmarks.search so it doesn't return the contents of tag folders, r?mak draft
authorBob Silverberg <bsilverberg@mozilla.com>
Mon, 24 Apr 2017 09:04:59 -0400
changeset 567280 b3fdfdfbdff1fa07e165c3402f060cd0f22b3e6c
parent 567252 62a2d3693579fc77b1c510984ae471a860d03302
child 625591 84ca36e7810b51ae726f788f34a25c1042bd9083
push id55513
push userbmo:bob.silverberg@gmail.com
push dateMon, 24 Apr 2017 18:00:46 +0000
reviewersmak
bugs1358127
milestone55.0a1
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();
+});