Bug 1421664 - 1 - Force any tag queries to have query result type bookmarks. r=mak
authormilindl <i.milind.luthra@gmail.com>
Tue, 20 Mar 2018 16:42:04 +0100
changeset 410768 7f8bb8f9998e0cd3a7c76e10bb3d47ec3c85fb6e
parent 410767 e29f8477cc6b6ae09c1222f851108607e259bee9
child 410769 4b1ce6bf8518e00a5c5125d905173d28a11e72e2
push id33739
push usernbeleuzu@mozilla.com
push dateFri, 30 Mar 2018 21:47:45 +0000
treeherdermozilla-central@10c662d8416e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1421664
milestone61.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 1421664 - 1 - Force any tag queries to have query result type bookmarks. r=mak MozReview-Commit-ID: KG1mGZD5PNy
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/queries/test_tags.js
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -1996,16 +1996,21 @@ nsNavHistory::ConstructQueryString(
 
     nsAutoCString additionalQueryOptions;
 
     queryString.ReplaceSubstring("{QUERY_OPTIONS}",
                                   additionalQueryOptions.get());
     return NS_OK;
   }
 
+  // If the query is a tag query, the type is bookmarks.
+  if (!aQuery->Tags().IsEmpty()) {
+    aOptions->SetQueryType(nsNavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS);
+  }
+
   nsAutoCString conditions;
   nsCString queryClause;
   rv = QueryToSelectClause(aQuery, aOptions, &queryClause);
   NS_ENSURE_SUCCESS(rv, rv);
   if (!queryClause.IsEmpty()) {
     // TODO: This should be set on a case basis, not blindly.
     aParamsPresent = true;
     conditions += queryClause;
--- a/toolkit/components/places/tests/queries/test_tags.js
+++ b/toolkit/components/places/tests/queries/test_tags.js
@@ -1,16 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
- * Tests bookmark and history queries with tags.  See bug 399799.
+ * Tests bookmark queries with tags.  See bug 399799.
  */
 
 "use strict";
 
 add_task(async function tags_getter_setter() {
   info("Tags getter/setter should work correctly");
   info("Without setting tags, tags getter should return empty array");
   var [query] = makeQuery();
@@ -138,166 +138,87 @@ add_task(async function() {
     "Abracadabra",
     "123",
     "Here's a pretty long tag name with some = signs and 1 2 3s and spaces oh jeez will it work I hope so!",
     "アスキーでございません",
     "あいうえお",
   ], true);
 });
 
-add_task(async function tag_to_uri() {
-  info("Querying history on tag associated with a URI should return " +
-       "that URI");
-  await task_doWithVisit(["foo", "bar", "baz"], function(aURI) {
+add_task(async function tag() {
+  info("Querying on tag associated with a URI should return that URI");
+  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
     var [query, opts] = makeQuery(["foo"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["bar"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["baz"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
   });
 });
 
-add_task(async function tags_to_uri() {
-  info("Querying history on many tags associated with a URI should " +
-       "return that URI");
-  await task_doWithVisit(["foo", "bar", "baz"], function(aURI) {
+add_task(async function many_tags() {
+  info("Querying on many tags associated with a URI should return that URI");
+  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
     var [query, opts] = makeQuery(["foo", "bar"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["foo", "baz"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["bar", "baz"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["foo", "bar", "baz"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
   });
 });
 
 add_task(async function repeated_tag() {
-  info("Specifying the same tag multiple times in a history query " +
-       "should not matter");
-  await task_doWithVisit(["foo", "bar", "baz"], function(aURI) {
+  info("Specifying the same tag multiple times should not matter");
+  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
     var [query, opts] = makeQuery(["foo", "foo"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["foo", "foo", "foo", "bar", "bar", "baz"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
   });
 });
 
-add_task(async function many_tags_no_uri() {
-  info("Querying history on many tags associated with a URI and " +
-       "tags not associated with that URI should not return that URI");
-  await task_doWithVisit(["foo", "bar", "baz"], function(aURI) {
+add_task(async function many_tags_no_bookmark() {
+  info("Querying on many tags associated with a URI and tags not associated " +
+       "with that URI should not return that URI");
+  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
     var [query, opts] = makeQuery(["foo", "bogus"]);
     executeAndCheckQueryResults(query, opts, []);
     [query, opts] = makeQuery(["foo", "bar", "bogus"]);
     executeAndCheckQueryResults(query, opts, []);
     [query, opts] = makeQuery(["foo", "bar", "baz", "bogus"]);
     executeAndCheckQueryResults(query, opts, []);
   });
 });
 
 add_task(async function nonexistent_tags() {
-  info("Querying history on nonexistent tags should return no results");
-  await task_doWithVisit(["foo", "bar", "baz"], function(aURI) {
+  info("Querying on nonexistent tag should return no results");
+  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
     var [query, opts] = makeQuery(["bogus"]);
     executeAndCheckQueryResults(query, opts, []);
     [query, opts] = makeQuery(["bogus", "gnarly"]);
     executeAndCheckQueryResults(query, opts, []);
   });
 });
 
-add_task(async function tag_to_bookmark() {
-  info("Querying bookmarks on tag associated with a URI should " +
-       "return that URI");
-  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
-    var [query, opts] = makeQuery(["foo"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["bar"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["baz"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-  });
-});
-
-add_task(async function many_tags_to_bookmark() {
-  info("Querying bookmarks on many tags associated with a URI " +
-       "should return that URI");
-  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
-    var [query, opts] = makeQuery(["foo", "bar"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["foo", "baz"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["bar", "baz"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["foo", "bar", "baz"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-  });
-});
-
-add_task(async function repeated_tag_to_bookmarks() {
-  info("Specifying the same tag multiple times in a bookmark query " +
-       "should not matter");
-  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
-    var [query, opts] = makeQuery(["foo", "foo"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["foo", "foo", "foo", "bar", "bar", "baz"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-  });
-});
-
-add_task(async function many_tags_no_bookmark() {
-  info("Querying bookmarks on many tags associated with a URI and " +
-       "tags not associated with that URI should not return that URI");
-  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
-    var [query, opts] = makeQuery(["foo", "bogus"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-    [query, opts] = makeQuery(["foo", "bar", "bogus"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-    [query, opts] = makeQuery(["foo", "bar", "baz", "bogus"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-  });
-});
-
-add_task(async function nonexistent_tags_bookmark() {
-  info("Querying bookmarks on nonexistent tag should return no results");
-  await task_doWithBookmark(["foo", "bar", "baz"], function(aURI) {
-    var [query, opts] = makeQuery(["bogus"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-    [query, opts] = makeQuery(["bogus", "gnarly"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-  });
-});
-
-add_task(async function tagsAreNot_history() {
-  info("Querying history using tagsAreNot should work correctly");
+add_task(async function tagsAreNot() {
+  info("Querying bookmarks using tagsAreNot should work correctly");
   var urisAndTags = {
     "http://example.com/1": ["foo", "bar"],
     "http://example.com/2": ["baz", "qux"],
     "http://example.com/3": null
   };
 
-  info("Add visits and tag the URIs");
+  info("Add bookmarks and tag the URIs");
   for (let [pURI, tags] of Object.entries(urisAndTags)) {
     let nsiuri = uri(pURI);
-    await PlacesTestUtils.addVisits(nsiuri);
+    await addBookmark(nsiuri);
     if (tags)
       PlacesUtils.tagging.tagURI(nsiuri, tags);
   }
 
   info('  Querying for "foo" should match only /2 and /3');
   var [query, opts] = makeQuery(["foo"], true);
   queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
                   ["http://example.com/2", "http://example.com/3"]);
@@ -328,73 +249,16 @@ add_task(async function tagsAreNot_histo
   for (let [pURI, tags] of Object.entries(urisAndTags)) {
     let nsiuri = uri(pURI);
     if (tags)
       PlacesUtils.tagging.untagURI(nsiuri, tags);
   }
   await task_cleanDatabase();
 });
 
-add_task(async function tagsAreNot_bookmarks() {
-  info("Querying bookmarks using tagsAreNot should work correctly");
-  var urisAndTags = {
-    "http://example.com/1": ["foo", "bar"],
-    "http://example.com/2": ["baz", "qux"],
-    "http://example.com/3": null
-  };
-
-  info("Add bookmarks and tag the URIs");
-  for (let [pURI, tags] of Object.entries(urisAndTags)) {
-    let nsiuri = uri(pURI);
-    await addBookmark(nsiuri);
-    if (tags)
-      PlacesUtils.tagging.tagURI(nsiuri, tags);
-  }
-
-  info('  Querying for "foo" should match only /2 and /3');
-  var [query, opts] = makeQuery(["foo"], true);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-  queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
-                  ["http://example.com/2", "http://example.com/3"]);
-
-  info('  Querying for "foo" and "bar" should match only /2 and /3');
-  [query, opts] = makeQuery(["foo", "bar"], true);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-  queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
-                  ["http://example.com/2", "http://example.com/3"]);
-
-  info('  Querying for "foo" and "bogus" should match only /2 and /3');
-  [query, opts] = makeQuery(["foo", "bogus"], true);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-  queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
-                  ["http://example.com/2", "http://example.com/3"]);
-
-  info('  Querying for "foo" and "baz" should match only /3');
-  [query, opts] = makeQuery(["foo", "baz"], true);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-  queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
-                  ["http://example.com/3"]);
-
-  info('  Querying for "bogus" should match all');
-  [query, opts] = makeQuery(["bogus"], true);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-  queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root,
-                  ["http://example.com/1",
-                   "http://example.com/2",
-                   "http://example.com/3"]);
-
-  // Clean up.
-  for (let [pURI, tags] of Object.entries(urisAndTags)) {
-    let nsiuri = uri(pURI);
-    if (tags)
-      PlacesUtils.tagging.untagURI(nsiuri, tags);
-  }
-  await task_cleanDatabase();
-});
-
 add_task(async function duplicate_tags() {
   info("Duplicate existing tags (i.e., multiple tag folders with " +
        "same name) should not throw off query results");
   var tagName = "foo";
 
   info("Add bookmark and tag it normally");
   await addBookmark(TEST_URI);
   PlacesUtils.tagging.tagURI(TEST_URI, [tagName]);
@@ -410,17 +274,16 @@ add_task(async function duplicate_tags()
   await PlacesUtils.bookmarks.insert({
     parentGuid: dupTag.guid,
     title: "title",
     url: TEST_URI
   });
 
   info("Querying for tag should match URI");
   var [query, opts] = makeQuery([tagName]);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
   queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root, [TEST_URI.spec]);
 
   PlacesUtils.tagging.untagURI(TEST_URI, [tagName]);
   await task_cleanDatabase();
 });
 
 add_task(async function folder_named_as_tag() {
   info("Regular folders with the same name as tag should not throw " +
@@ -435,17 +298,16 @@ add_task(async function folder_named_as_
   await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     type: PlacesUtils.bookmarks.TYPE_FOLDER,
     title: tagName
   });
 
   info("Querying for tag should match URI");
   var [query, opts] = makeQuery([tagName]);
-  opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
   queryResultsAre(PlacesUtils.history.executeQuery(query, opts).root, [TEST_URI.spec]);
 
   PlacesUtils.tagging.untagURI(TEST_URI, [tagName]);
   await task_cleanDatabase();
 });
 
 add_task(async function ORed_queries() {
   info("Multiple queries ORed together should work");
@@ -456,97 +318,61 @@ add_task(async function ORed_queries() {
 
   // Search with lots of tags to make sure tag parameter substitution in SQL
   // can handle it with more than one query.
   for (let i = 0; i < 11; i++) {
     urisAndTags["http://example.com/1"].push("/1 tag " + i);
     urisAndTags["http://example.com/2"].push("/2 tag " + i);
   }
 
-  info("Add visits and tag the URIs");
+  info("Add bookmarks and tag the URIs");
   for (let [pURI, tags] of Object.entries(urisAndTags)) {
     let nsiuri = uri(pURI);
-    await PlacesTestUtils.addVisits(nsiuri);
+    await addBookmark(nsiuri);
     if (tags)
       PlacesUtils.tagging.tagURI(nsiuri, tags);
   }
 
   // Clean up.
   for (let [pURI, tags] of Object.entries(urisAndTags)) {
     let nsiuri = uri(pURI);
     if (tags)
       PlacesUtils.tagging.untagURI(nsiuri, tags);
   }
   await task_cleanDatabase();
 });
 
-add_task(async function tag_casing_uri() {
-  info("Querying history on associated tags should return " +
+add_task(async function tag_casing() {
+  info("Querying on associated tags should return " +
        "correct results irrespective of casing of tags.");
-  await task_doWithVisit(["fOo", "bAr"], function(aURI) {
+  await task_doWithBookmark(["fOo", "bAr"], function(aURI) {
     let [query, opts] = makeQuery(["Foo"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["Foo", "Bar"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["Foo"], true);
     executeAndCheckQueryResults(query, opts, []);
     [query, opts] = makeQuery(["Bogus"], true);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
   });
 });
 
-add_task(async function tag_casing_bookmark() {
-  info("Querying bookmarks on associated tags should return " +
-       "correct results irrespective of casing of tags.");
-  await task_doWithBookmark(["fOo", "bAr"], function(aURI) {
-    let [query, opts] = makeQuery(["Foo"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["Foo", "Bar"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    [query, opts] = makeQuery(["Foo"], true);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-    [query, opts] = makeQuery(["Bogus"], true);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-  });
-});
-
-add_task(async function tag_special_char_uri() {
-  info("Querying history on associated tags should return " +
+add_task(async function tag_special_char() {
+  info("Querying on associated tags should return " +
        "correct results even if tags contain special characters.");
-  await task_doWithVisit(["Space ☺️ Between"], function(aURI) {
+  await task_doWithBookmark(["Space ☺️ Between"], function(aURI) {
     let [query, opts] = makeQuery(["Space ☺️ Between"]);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
     [query, opts] = makeQuery(["Space ☺️ Between"], true);
     executeAndCheckQueryResults(query, opts, []);
     [query, opts] = makeQuery(["Bogus"], true);
     executeAndCheckQueryResults(query, opts, [aURI.spec]);
   });
 });
 
-add_task(async function tag_special_char_bookmark() {
-  info("Querying bookmarks on associated tags should return " +
-       "correct results even if tags contain special characters.");
-  await task_doWithBookmark(["Space ☺️ Between"], function(aURI) {
-    let [query, opts] = makeQuery(["Space ☺️ Between"]);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-    [query, opts] = makeQuery(["Space ☺️ Between"], true);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, []);
-    [query, opts] = makeQuery(["Bogus"], true);
-    opts.queryType = opts.QUERY_TYPE_BOOKMARKS;
-    executeAndCheckQueryResults(query, opts, [aURI.spec]);
-  });
-});
-
 // The tag keys in query URIs, i.e., "place:tag=foo&!tags=1"
 //                                          ---     -----
 const QUERY_KEY_TAG      = "tag";
 const QUERY_KEY_NOT_TAGS = "!tags";
 
 const TEST_URI = uri("http://example.com/");
 
 /**
@@ -605,34 +431,16 @@ async function task_doWithBookmark(aTags
   await addBookmark(TEST_URI);
   PlacesUtils.tagging.tagURI(TEST_URI, aTags);
   await aCallback(TEST_URI);
   PlacesUtils.tagging.untagURI(TEST_URI, aTags);
   await task_cleanDatabase();
 }
 
 /**
- * Asynchronous task that executes a callback function in a "scoped" database
- * state.  A history visit is added and tagged before the callback is called,
- * and afterward the database is cleared.
- *
- * @param aTags
- *        A history visit will be added and tagged with this array of tags
- * @param aCallback
- *        A function that will be called after the visit has been tagged
- */
-async function task_doWithVisit(aTags, aCallback) {
-  await PlacesTestUtils.addVisits(TEST_URI);
-  PlacesUtils.tagging.tagURI(TEST_URI, aTags);
-  await aCallback(TEST_URI);
-  PlacesUtils.tagging.untagURI(TEST_URI, aTags);
-  await task_cleanDatabase();
-}
-
-/**
  * queryToQueryString() encodes every character in the query URI that doesn't
  * match /[a-zA-Z]/.  There's no simple JavaScript function that does the same,
  * but encodeURIComponent() comes close, only missing some punctuation.  This
  * function takes care of all of that.
  *
  * @param  aTag
  *         A tag name to encode
  * @return A UTF-8 escaped string suitable for inclusion in a query URI