From 327e8fc60129c98b2798e3016323c48ee173b7d6 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Thu, 5 Oct 2017 15:13:36 +0200 Subject: [PATCH] Bug 1402555 - deleting history from urlbar completion list doesn't work. r=adw MozReview-Commit-ID: 6W0Nz9N8DtJ --HG-- extra : rebase_source : e8fcf71b2f560eebda7a1864f93d0770771042ce --- browser/base/content/test/urlbar/browser.ini | 1 + .../urlbar/browser_urlbar_remove_match.js | 28 ++++++++++++++++++ toolkit/components/places/UnifiedComplete.js | 29 ++++++++----------- 3 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 browser/base/content/test/urlbar/browser_urlbar_remove_match.js diff --git a/browser/base/content/test/urlbar/browser.ini b/browser/base/content/test/urlbar/browser.ini index 7ae3b0912916..358179eb5836 100644 --- a/browser/base/content/test/urlbar/browser.ini +++ b/browser/base/content/test/urlbar/browser.ini @@ -113,6 +113,7 @@ support-files = [browser_urlbar_locationchange_urlbar_edit_dos.js] support-files = file_urlbar_edit_dos.html +[browser_urlbar_remove_match.js] [browser_urlbar_searchsettings.js] [browser_urlbar_search_speculative_connect.js] [browser_urlbar_search_speculative_connect_engine.js] diff --git a/browser/base/content/test/urlbar/browser_urlbar_remove_match.js b/browser/base/content/test/urlbar/browser_urlbar_remove_match.js new file mode 100644 index 000000000000..ad75022075bf --- /dev/null +++ b/browser/base/content/test/urlbar/browser_urlbar_remove_match.js @@ -0,0 +1,28 @@ +/* eslint-disable mozilla/no-arbitrary-setTimeout */ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +add_task(async function test_remove_history() { + const TEST_URL = "http://remove.me/from_urlbar/"; + await PlacesTestUtils.addVisits(TEST_URL); + + registerCleanupFunction(async function() { + await PlacesUtils.history.clear(); + }); + + let promiseVisitRemoved = PlacesTestUtils.waitForNotification( + "onDeleteURI", uri => uri.spec == TEST_URL, "history"); + await promiseAutocompleteResultPopup("remove.me/from_urlbar"); + await BrowserTestUtils.waitForCondition( + () => gURLBar.popup.richlistbox.children.length > 1 && + gURLBar.popup.richlistbox.children[1].getAttribute("ac-value") == TEST_URL, + "Waiting for the result to appear"); + EventUtils.synthesizeKey("VK_DOWN", {}); + Assert.equal(gURLBar.popup.richlistbox.selectedIndex, 1); + let options = AppConstants.platform == "macosx" ? { shiftKey: true } : {}; + EventUtils.synthesizeKey("VK_DELETE", options); + await promiseVisitRemoved; + await BrowserTestUtils.waitForCondition( + () => !gURLBar.popup.richlistbox.children.some(c => !c.collapsed && c.getAttribute("ac-value") == TEST_URL), + "Waiting for the result to disappear"); +}); diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index b161664b9496..52bb560efaae 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -788,8 +788,6 @@ function looksLikeUrl(str, ignoreAlphanumericHosts = false) { * * user-context-id: The userContextId of the selected tab. * @param autocompleteListener * An nsIAutoCompleteObserver. - * @param resultListener - * An nsIAutoCompleteSimpleResultListener. * @param autocompleteSearch * An nsIAutoCompleteSearch. * @param prohibitSearchSuggestions @@ -798,8 +796,7 @@ function looksLikeUrl(str, ignoreAlphanumericHosts = false) { * The result object from the previous search. if available. */ function Search(searchString, searchParam, autocompleteListener, - resultListener, autocompleteSearch, prohibitSearchSuggestions, - previousResult) { + autocompleteSearch, prohibitSearchSuggestions, previousResult) { // We want to store the original string for case sensitive searches. this._originalSearchString = searchString; this._trimmedOriginalSearchString = searchString.trim(); @@ -844,7 +841,16 @@ function Search(searchString, searchParam, autocompleteListener, Cc["@mozilla.org/autocomplete/simple-result;1"] .createInstance(Ci.nsIAutoCompleteSimpleResult); result.setSearchString(searchString); - result.setListener(resultListener); + result.setListener({ + onValueRemoved(result, spec, removeFromDB) { + if (removeFromDB) { + PlacesUtils.history.remove(spec).catch(Cu.reportError); + } + }, + QueryInterface: XPCOMUtils.generateQI([ + Ci.nsIAutoCompleteSimpleResultListener + ]) + }); // Will be set later, if needed. result.setDefaultIndex(-1); this._result = result; @@ -2374,7 +2380,6 @@ Search.prototype = { this._listener.onSearchResult(this._autocompleteSearch, result); if (!searchOngoing) { // Break possible cycles. - this._result.setListener(null); this._listener = null; this._autocompleteSearch = null; } @@ -2490,7 +2495,6 @@ UnifiedComplete.prototype = { searchString.length > this._lastLowResultsSearchSuggestion.length && searchString.startsWith(this._lastLowResultsSearchSuggestion); - // We don't directly reuse the controller provided previousResult because: // * it is only populated when the new searchString is an extension of the // previous one. We want to handle the backspace case too. @@ -2524,7 +2528,7 @@ UnifiedComplete.prototype = { } this._currentSearch = new Search(searchString, searchParam, listener, - this, this, prohibitSearchSuggestions, + this, prohibitSearchSuggestions, previousResult); // If we are not enabled, we need to return now. Notice we need an empty @@ -2593,14 +2597,6 @@ UnifiedComplete.prototype = { search.notifyResults(false); }, - // nsIAutoCompleteSimpleResultListener - - onValueRemoved(result, spec, removeFromDB) { - if (removeFromDB) { - PlacesUtils.history.remove(spec).catch(Cu.reportError); - } - }, - // nsIAutoCompleteSearchDescriptor get searchType() { @@ -2619,7 +2615,6 @@ UnifiedComplete.prototype = { QueryInterface: XPCOMUtils.generateQI([ Ci.nsIAutoCompleteSearch, - Ci.nsIAutoCompleteSimpleResultListener, Ci.nsIAutoCompleteSearchDescriptor, Ci.mozIPlacesAutoComplete, Ci.nsIObserver,