From 6773e5bbff0963018c20bd1a1c79568160bf2576 Mon Sep 17 00:00:00 2001 From: Shawn Wilsher Date: Wed, 23 Sep 2009 10:19:27 -0700 Subject: [PATCH] Bug 516465 - Adaptive results aren't filtered. r=dietrich Run adaptive search results through AUTOCOMPLETE_MATCH so they properly obey the preferences of the user. Also runs through adaptive results again if we do not have enough matches and the match behavior is MATCH_BOUNDARY_ANYWHERE. --- .../places/src/nsPlacesAutoComplete.js | 22 ++- .../places/tests/unit/test_adaptive.js | 147 ++++++++++++++---- 2 files changed, 133 insertions(+), 36 deletions(-) diff --git a/toolkit/components/places/src/nsPlacesAutoComplete.js b/toolkit/components/places/src/nsPlacesAutoComplete.js index bec5bea393d..2f2a98611fe 100644 --- a/toolkit/components/places/src/nsPlacesAutoComplete.js +++ b/toolkit/components/places/src/nsPlacesAutoComplete.js @@ -322,6 +322,10 @@ function nsPlacesAutoComplete() "LEFT JOIN moz_places_temp h_t ON h_t.id = i.place_id " + "LEFT JOIN moz_favicons f ON f.id = IFNULL(h_t.favicon_id, h.favicon_id) "+ "WHERE IFNULL(h_t.url, h.url) NOTNULL " + + "AND AUTOCOMPLETE_MATCH(:searchString, 0 /* url */, " + + "IFNULL(bookmark, 1 /* title */), tags, " + + "6 /* visit_count */, 7 /* typed */, parent, " + + ":matchBehavior, :searchBehavior) " + "ORDER BY rank DESC, IFNULL(h_t.frecency, h.frecency) DESC" ); }); @@ -484,8 +488,11 @@ nsPlacesAutoComplete.prototype = { if (this._matchBehavior == MATCH_BOUNDARY_ANYWHERE && this._result.matchCount < this._maxRichResults && !this._secondPass) { this._secondPass = true; - let query = this._getBoundSearchQuery(MATCH_ANYWHERE, this._searchTokens); - this._executeQueries([query]); + let queries = [ + this._getBoundAdaptiveQuery(MATCH_ANYWHERE), + this._getBoundSearchQuery(MATCH_ANYWHERE, this._searchTokens), + ]; + this._executeQueries(queries); return; } @@ -827,13 +834,19 @@ nsPlacesAutoComplete.prototype = { * * @return the bound adaptive query. */ - _getBoundAdaptiveQuery: function PAC_getBoundAdaptiveQuery() + _getBoundAdaptiveQuery: function PAC_getBoundAdaptiveQuery(aMatchBehavior) { + // If we were not given a match behavior, use the stored match behavior. + if (arguments.length == 0) + aMatchBehavior = this._matchBehavior; + let query = this._adaptiveQuery; let (params = query.params) { params.parent = this._bs.tagsFolder; params.search_string = this._currentSearchString; params.query_type = kQueryTypeFiltered; + params.matchBehavior = aMatchBehavior; + params.searchBehavior = this._behavior; } return query; @@ -898,7 +911,8 @@ nsPlacesAutoComplete.prototype = { // the result does not fall into any of those, it just gets the favicon. if (!style) { // It is possible that we already have a style set (from a keyword - // search), so only set it if we haven't already done so. + // search or because of the user's preferences), so only set it if we + // haven't already done so. if (showTags) style = "tag"; else if (entryParentId) diff --git a/toolkit/components/places/tests/unit/test_adaptive.js b/toolkit/components/places/tests/unit/test_adaptive.js index bf1fc8ead91..a30f9e355ab 100644 --- a/toolkit/components/places/tests/unit/test_adaptive.js +++ b/toolkit/components/places/tests/unit/test_adaptive.js @@ -58,8 +58,15 @@ Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); let histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. getService(Ci.nsINavHistoryService); let bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory); +let bsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. + getService(Ci.nsINavBookmarksService); +let tsvc = Cc["@mozilla.org/browser/tagging-service;1"]. + getService(Ci.nsITaggingService); let obs = Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService); +let prefs = Cc["@mozilla.org/preferences-service;1"]. + getService(Ci.nsIPrefBranch); + const PLACES_AUTOCOMPLETE_FEEDBACK_UPDATED_TOPIC = "places-autocomplete-feedback-updated"; @@ -119,7 +126,7 @@ AutoCompleteInput.prototype = { /** * Checks that autocomplete results are ordered correctly */ -function ensure_results(uris, searchTerm) +function ensure_results(expected, searchTerm) { let controller = Components.classes["@mozilla.org/autocomplete/controller;1"]. getService(Components.interfaces.nsIAutoCompleteController); @@ -133,9 +140,11 @@ function ensure_results(uris, searchTerm) input.onSearchComplete = function() { do_check_eq(controller.searchStatus, Ci.nsIAutoCompleteController.STATUS_COMPLETE_MATCH); - do_check_eq(controller.matchCount, uris.length); + do_check_eq(controller.matchCount, expected.length); for (let i = 0; i < controller.matchCount; i++) { - do_check_eq(controller.getValueAt(i), uris[i].spec); + print("Testing for '" + expected[i].uri.spec + "' got '" + controller.getValueAt(i) + "'"); + do_check_eq(controller.getValueAt(i), expected[i].uri.spec); + do_check_eq(controller.getStyleAt(i), expected[i].style); } if (tests.length) @@ -152,7 +161,7 @@ function ensure_results(uris, searchTerm) /** * Bump up the rank for an uri */ -function setCountRank(aURI, aCount, aRank, aSearch) +function setCountRank(aURI, aCount, aRank, aSearch, aBookmark) { // Bump up the visit count for the uri for (let i = 0; i < aCount; i++) @@ -175,6 +184,16 @@ function setCountRank(aURI, aCount, aRank, aSearch) for (let i = 0; i < aRank; i++) { obs.notifyObservers(thing, "autocomplete-will-enter-text", null); } + + // If this is supposed to be a bookmark, add it. + if (aBookmark) { + bsvc.insertBookmark(bsvc.unfiledBookmarksFolder, aURI, bsvc.DEFAULT_INDEX, + "test_book"); + + // And add the tag if we need to. + if (aBookmark == "tag") + tsvc.tagURI(aURI, "test_tag"); + } } /** @@ -200,15 +219,14 @@ let s1 = "si"; let s2 = "site"; let observer = { - uriA: null, - uriB: null, + results: null, search: null, runCount: -1, observe: function(aSubject, aTopic, aData) { if (PLACES_AUTOCOMPLETE_FEEDBACK_UPDATED_TOPIC == aTopic && !(--this.runCount)) { - ensure_results([this.uriA, this.uriB], this.search); + ensure_results(this.results, this.search); } } }; @@ -221,14 +239,30 @@ function prepTest(name) { print("Test " + name); bhist.removeAllPages(); observer.runCount = -1; + + // Remove all bookmarks and tags. + bsvc.removeFolderChildren(bsvc.unfiledBookmarksFolder); + bsvc.removeFolderChildren(bsvc.tagsFolder); +} + +/** + * Make the result object for a given URI that will be passed to ensure_results. + */ +function makeResult(aURI) { + return { + uri: aURI, + style: "favicon", + }; } let tests = [ // Test things without a search term function() { prepTest("0 same count, diff rank, same term; no search"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s0; observer.runCount = c1 + c2; setCountRank(uri1, c1, c1, s2); @@ -236,8 +270,10 @@ let tests = [ }, function() { prepTest("1 same count, diff rank, same term; no search"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s0; observer.runCount = c1 + c2; setCountRank(uri1, c1, c2, s2); @@ -245,8 +281,10 @@ let tests = [ }, function() { prepTest("2 diff count, same rank, same term; no search"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s0; observer.runCount = c1 + c1; setCountRank(uri1, c1, c1, s2); @@ -254,8 +292,10 @@ let tests = [ }, function() { prepTest("3 diff count, same rank, same term; no search"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s0; observer.runCount = c1 + c1; setCountRank(uri1, c2, c1, s2); @@ -265,8 +305,10 @@ let tests = [ // Test things with a search term (exact match one, partial other) function() { prepTest("4 same count, same rank, diff term; one exact/one partial search"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s1; observer.runCount = c1 + c1; setCountRank(uri1, c1, c1, s1); @@ -274,8 +316,10 @@ let tests = [ }, function() { prepTest("5 same count, same rank, diff term; one exact/one partial search"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s1; observer.runCount = c1 + c1; setCountRank(uri1, c1, c1, s2); @@ -285,8 +329,10 @@ let tests = [ // Test things with a search term (exact match both) function() { prepTest("6 same count, diff rank, same term; both exact search"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s1; observer.runCount = c1 + c2; setCountRank(uri1, c1, c1, s1); @@ -294,8 +340,10 @@ let tests = [ }, function() { prepTest("7 same count, diff rank, same term; both exact search"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s1; observer.runCount = c1 + c2; setCountRank(uri1, c1, c2, s1); @@ -305,8 +353,10 @@ let tests = [ // Test things with a search term (partial match both) function() { prepTest("8 same count, diff rank, same term; both partial search"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s1; observer.runCount = c1 + c2; setCountRank(uri1, c1, c1, s2); @@ -314,8 +364,10 @@ let tests = [ }, function() { prepTest("9 same count, diff rank, same term; both partial search"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s1; observer.runCount = c1 + c2; setCountRank(uri1, c1, c2, s2); @@ -323,8 +375,10 @@ let tests = [ }, function() { prepTest("10 same count, same rank, same term, decay first; exact match"); - observer.uriA = uri2; - observer.uriB = uri1; + observer.results = [ + makeResult(uri2), + makeResult(uri1), + ]; observer.search = s1; observer.runCount = c1 + c1; setCountRank(uri1, c1, c1, s1); @@ -333,18 +387,47 @@ let tests = [ }, function() { prepTest("11 same count, same rank, same term, decay second; exact match"); - observer.uriA = uri1; - observer.uriB = uri2; + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; observer.search = s1; observer.runCount = c1 + c1; setCountRank(uri2, c1, c1, s1); doAdaptiveDecay(); setCountRank(uri1, c1, c1, s1); }, + // Test that bookmarks or tags are hidden if the preferences are set right. + function() { + prepTest("12 same count, diff rank, same term; no search; history only"); + prefs.setIntPref("browser.urlbar.matchBehavior", + Ci.mozIPlacesAutoComplete.BEHAVIOR_HISTORY); + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; + observer.search = s0; + observer.runCount = c1 + c2; + setCountRank(uri1, c1, c1, s2, "bookmark"); + setCountRank(uri2, c1, c2, s2); + }, + function() { + prepTest("13 same count, diff rank, same term; no search; history only with tag"); + prefs.setIntPref("browser.urlbar.matchBehavior", + Ci.mozIPlacesAutoComplete.BEHAVIOR_HISTORY); + observer.results = [ + makeResult(uri1), + makeResult(uri2), + ]; + observer.search = s0; + observer.runCount = c1 + c2; + setCountRank(uri1, c1, c1, s2, "tag"); + setCountRank(uri2, c1, c2, s2); + }, ]; /** - * Test adapative autocomplete + * Test adaptive autocomplete */ function run_test() { do_test_pending();