Bug 485122 - When the user selects to only display history in the location bar, don't display the star and tag icons for items that happen to be bookmarked or tagged. r=dietrich
authorEdward Lee <edilee@mozilla.com>
Tue, 12 May 2009 10:17:57 -0500
changeset 28237 033f19d61c13622d44e06094b31e8bd98fddb35e
parent 28236 6c48080dba78e9aa444aa70212e0806cf51743dc
child 28246 5268bddbe0a1d191bd2c728d374dd8183de73dea
push id6964
push useredward.lee@engineering.uiuc.edu
push dateTue, 12 May 2009 15:18:23 +0000
treeherdermozilla-central@033f19d61c13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdietrich
bugs485122
milestone1.9.2a1pre
Bug 485122 - When the user selects to only display history in the location bar, don't display the star and tag icons for items that happen to be bookmarked or tagged. r=dietrich Pretend a page isn't bookmarked/tagged when searching for only history unless the user explicitly wants them. Test by updating special searches test to ignore tags when searching for history ^.
toolkit/components/places/src/nsNavHistoryAutoComplete.cpp
toolkit/components/places/tests/autocomplete/head_autocomplete.js
toolkit/components/places/tests/autocomplete/test_special_search.js
--- a/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp
+++ b/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp
@@ -1136,16 +1136,23 @@ nsNavHistory::AutoCompleteProcessSearch(
 
           break;
         }
       }
 
       // Always prefer to show tags if we have them
       PRBool showTags = !entryTags.IsEmpty();
 
+      // Pretend a page isn't bookmarked/tagged if the user only wants history,
+      // but still show the star and tag if the user explicitly wants them
+      if (GET_BEHAVIOR(History) && !(GET_BEHAVIOR(Bookmark) || GET_BEHAVIOR(Tag))) {
+        showTags = PR_FALSE;
+        style = NS_LITERAL_STRING("favicon");
+      }
+
       // Add the tags to the title if necessary
       if (showTags)
         title += TITLE_TAGS_SEPARATOR + entryTags;
 
       // Tags have a special style to show a tag icon; otherwise, style the
       // bookmarks that aren't feed items and feed URIs as bookmark
       if (style.IsEmpty()) {
         if (showTags)
--- a/toolkit/components/places/tests/autocomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/autocomplete/head_autocomplete.js
@@ -74,16 +74,24 @@ AutoCompleteInput.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIAutoCompleteInput, Ci.nsIAutoCompletePopup])
 };
 
 function toURI(aSpec)
 {
   return iosvc.newURI(aSpec, null, null);
 }
 
+let appendTags = true;
+// Helper to turn off tag matching in results
+function ignoreTags()
+{
+  print("Ignoring tags from results");
+  appendTags = false;
+}
+
 function ensure_results(aSearch, aExpected)
 {
   let controller = Cc["@mozilla.org/autocomplete/controller;1"].
                    getService(Ci.nsIAutoCompleteController);
 
   // Make an AutoCompleteInput that uses our searches
   // and confirms results on search complete
   let input = new AutoCompleteInput(["history"]);
@@ -111,17 +119,17 @@ function ensure_results(aSearch, aExpect
         // Skip processed expected results
         if (aExpected[j] == undefined) continue;
 
         let [uri, title, tags] = gPages[aExpected[j]];
 
         // Load the real uri and titles and tags if necessary
         uri = toURI(kURIs[uri]).spec;
         title = kTitles[title];
-        if (tags)
+        if (tags && appendTags)
           title += " \u2013 " + tags.map(function(aTag) kTitles[aTag]);
 
         // Got a match on both uri and title?
         if (uri == value && title == comment) {
           print("Got it at index " + j + "!!");
           // Make it undefined so we don't process it again
           aExpected[j] = undefined;
           break;
@@ -344,16 +352,19 @@ function run_test() {
 
   // Search is asynchronous, so don't let the test finish immediately
   do_test_pending();
 
   // Load the test and print a description then run the test
   let [description, search, expected, func] = gTests[current_test];
   print(description);
 
+  // By default assume we want to match tags
+  appendTags = true;
+
   // Do an extra function if necessary
   if (func)
     func();
 
   ensure_results(search, expected);
 }
 
 // Utility function to remove history pages
@@ -365,8 +376,9 @@ function removePages(aURIs)
 
 // Utility function to mark pages as typed
 function markTyped(aURIs)
 {
   for each (let uri in aURIs)
     histsvc.addVisit(toURI(kURIs[uri]), Date.now() * 1000, null,
       histsvc.TRANSITION_TYPED, false, 0);
 }
+
--- a/toolkit/components/places/tests/autocomplete/test_special_search.js
+++ b/toolkit/components/places/tests/autocomplete/test_special_search.js
@@ -32,16 +32,19 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 /**
  * Test for bug 395161 that allows special searches that restrict results to
  * history/bookmark/tagged items and title/url matches.
+ *
+ * Test 485122 by making sure results don't have tags when restricting result
+ * to just history either by default behavior or dynamic query restrict.
  */
 
 // Define some shared uris and titles (each page needs its own uri)
 let kURIs = [
   "http://url/",
   "http://url/2",
   "http://foo.bar/",
   "http://foo.bar/2",
@@ -83,55 +86,55 @@ removePages([4,6,7,8,9,11]);
 // Set some pages as typed
 markTyped([0,3,10]);
 
 // Provide for each test: description; search terms; array of gPages indices of
 // pages that should match; optional function to be run before the test
 let gTests = [
   // Test restricting searches
   ["0: History restrict",
-   "^", [0,1,2,3,5,10]],
+   "^", [0,1,2,3,5,10], ignoreTags],
   ["1: Star restrict",
    "*", [4,5,6,7,8,9,10,11]],
   ["2: Tag restrict",
    "+", [8,9,10,11]],
 
   // Test specials as any word position
   ["3: Special as first word",
-   "^ foo bar", [1,2,3,5,10]],
+   "^ foo bar", [1,2,3,5,10], ignoreTags],
   ["4: Special as middle word",
-   "foo ^ bar", [1,2,3,5,10]],
+   "foo ^ bar", [1,2,3,5,10], ignoreTags],
   ["5: Special as last word",
-   "foo bar ^", [1,2,3,5,10]],
+   "foo bar ^", [1,2,3,5,10], ignoreTags],
 
   // Test restricting and matching searches with a term
   ["6: foo ^ -> history",
-   "foo ^", [1,2,3,5,10]],
+   "foo ^", [1,2,3,5,10], ignoreTags],
   ["7: foo * -> is star",
    "foo *", [5,6,7,8,9,10,11]],
   ["8: foo # -> in title",
    "foo #", [1,3,5,7,8,9,10,11]],
   ["9: foo @ -> in url",
    "foo @", [2,3,6,7,10,11]],
   ["10: foo + -> is tag",
    "foo +", [8,9,10,11]],
   ["10.1: foo ~ -> is typed",
    "foo ~", [3,10]],
 
   // Test various pairs of special searches
   ["11: foo ^ * -> history, is star",
    "foo ^ *", [5,10]],
   ["12: foo ^ # -> history, in title",
-   "foo ^ #", [1,3,5,10]],
+   "foo ^ #", [1,3,5,10], ignoreTags],
   ["13: foo ^ @ -> history, in url",
-   "foo ^ @", [2,3,10]],
+   "foo ^ @", [2,3,10], ignoreTags],
   ["14: foo ^ + -> history, is tag",
    "foo ^ +", [10]],
   ["14.1: foo ^ ~ -> history, is typed",
-   "foo ^ ~", [3,10]],
+   "foo ^ ~", [3,10], ignoreTags],
   ["15: foo * # -> is star, in title",
    "foo * #", [5,7,8,9,10,11]],
   ["16: foo * @ -> is star, in url",
    "foo * @", [6,7,10,11]],
   ["17: foo * + -> same as +",
    "foo * +", [8,9,10,11]],
   ["17.1: foo * ~ -> is star, is typed",
    "foo * ~", [10]],
@@ -161,10 +164,14 @@ let gTests = [
   // Change the default to be less restrictive to make sure we find more
   ["24: foo -> default is star, in url",
    "foo", [6,7,10,11], function() makeDefault(18)],
   ["25: foo -> default in url",
    "foo", [2,3,6,7,10,11], function() makeDefault(16)],
 ];
 
 function makeDefault(aDefault) {
+  // We want to ignore tags if we're restricting to history unless we're showing
+  if ((aDefault & 1) && !((aDefault & 2) || (aDefault & 4)))
+    ignoreTags();
+
   prefs.setIntPref("browser.urlbar.default.behavior", aDefault);
 }