Bug 1506126 - Support clearing history for selected entries. r=dao
authorMark Banner <standard8@mozilla.com>
Tue, 15 Jan 2019 12:20:48 +0000
changeset 513900 4234a4516c4526b942e635e825a259206287ac7f
parent 513899 a8031a63f5971d5caff0f6874447211dd55c40e0
child 513901 5090d461e169e160de477c06b3606c9e130f9f00
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1506126
milestone66.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 1506126 - Support clearing history for selected entries. r=dao Differential Revision: https://phabricator.services.mozilla.com/D16332
browser/base/content/test/urlbar/browser_urlbar_remove_match.js
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarView.jsm
--- a/browser/base/content/test/urlbar/browser_urlbar_remove_match.js
+++ b/browser/base/content/test/urlbar/browser_urlbar_remove_match.js
@@ -1,9 +1,8 @@
-/* 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() {
@@ -24,8 +23,42 @@ add_task(async function test_remove_hist
   await promiseVisitRemoved;
   await BrowserTestUtils.waitForCondition(
     () => !gURLBar.popup.richlistbox.itemChildren.some(c => !c.collapsed && c.getAttribute("ac-value") == TEST_URL),
     "Waiting for the result to disappear");
 
   gURLBar.popup.hidePopup();
   await promisePopupHidden(gURLBar.popup);
 });
+
+// We shouldn't be able to remove a bookmark item.
+add_task(async function test_remove_bookmark_doesnt() {
+  const TEST_URL = "http://dont.remove.me/from_urlbar/";
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    title: "test",
+    url: TEST_URL,
+  });
+
+  registerCleanupFunction(async function() {
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+
+  await promiseAutocompleteResultPopup("from_urlbar");
+  let result = await waitForAutocompleteResultAt(1);
+  Assert.equal(result.getAttribute("ac-value"), TEST_URL, "Found the expected result");
+
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  Assert.equal(gURLBar.popup.richlistbox.selectedIndex, 1);
+  let options = AppConstants.platform == "macosx" ? { shiftKey: true } : {};
+  EventUtils.synthesizeKey("KEY_Delete", options);
+
+  // We don't have an easy way of determining if the event was process or not,
+  // so let any event queues clear before testing.
+  await new Promise(resolve => setTimeout(resolve, 0));
+  await PlacesTestUtils.promiseAsyncUpdates();
+
+  gURLBar.popup.hidePopup();
+  await promisePopupHidden(gURLBar.popup);
+
+  Assert.ok(await PlacesUtils.bookmarks.fetch({url: TEST_URL}),
+    "Should still have the URL bookmarked.");
+});
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -5,34 +5,37 @@
 "use strict";
 
 var EXPORTED_SYMBOLS = ["QueryContext", "UrlbarController"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetters(this, {
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   // BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
+  UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
 const TELEMETRY_1ST_RESULT = "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS";
 const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
 
 /**
  * The address bar controller handles queries from the address bar, obtains
  * results and returns them to the UI for display.
  *
  * Listeners may be added to listen for the results. They must support the
  * following methods which may be called when a query is run:
  *
  * - onQueryStarted(queryContext)
  * - onQueryResults(queryContext)
  * - onQueryCancelled(queryContext)
  * - onQueryFinished(queryContext)
+ * - onQueryResultRemoved(index)
  */
 class UrlbarController {
   /**
    * Initialises the class. The manager may be overridden here, this is for
    * test purposes.
    *
    * @param {object} options
    *   The initial options for UrlbarController.
@@ -170,33 +173,34 @@ class UrlbarController {
   /**
    * Receives keyboard events from the input and handles those that should
    * navigate within the view or pick the currently selected item.
    *
    * @param {KeyboardEvent} event
    *   The DOM KeyboardEvent.
    */
   handleKeyNavigation(event) {
+    const isMac = AppConstants.platform == "macosx";
     // Handle readline/emacs-style navigation bindings on Mac.
-    if (AppConstants.platform == "macosx" &&
+    if (isMac &&
         this.view.isOpen &&
         event.ctrlKey &&
         (event.key == "n" || event.key == "p")) {
       this.view.selectNextItem({ reverse: event.key == "p" });
       event.preventDefault();
       return;
     }
 
     switch (event.keyCode) {
       case KeyEvent.DOM_VK_ESCAPE:
         this.input.handleRevert();
         event.preventDefault();
         break;
       case KeyEvent.DOM_VK_RETURN:
-        if (AppConstants.platform == "macosx" &&
+        if (isMac &&
             event.metaKey) {
           // Prevent beep on Mac.
           event.preventDefault();
         }
         // TODO: We may have an autoFill entry, so we should use that instead.
         // TODO: We should have an input bufferrer so that we can use search results
         // if appropriate.
         this.input.handleCommand(event);
@@ -214,20 +218,65 @@ class UrlbarController {
             this.view.selectNextItem({
               reverse: event.keyCode == KeyEvent.DOM_VK_UP });
           } else {
             this.input.startQuery();
           }
           event.preventDefault();
         }
         break;
+      case KeyEvent.DOM_VK_DELETE:
+        if (isMac && !event.shiftKey) {
+          break;
+        }
+        if (this._handleDeleteEntry()) {
+          event.preventDefault();
+        }
+        break;
+      case KeyEvent.DOM_VK_BACK_SPACE:
+        if (isMac && event.shiftKey &&
+            this._handleDeleteEntry()) {
+          event.preventDefault();
+        }
+        break;
     }
   }
 
   /**
+   * Internal function handling deletion of entries. We only support removing
+   * of history entries - other result sources will be ignored.
+   *
+   * @returns {boolean} Returns true if the deletion was acted upon.
+   */
+  _handleDeleteEntry() {
+    if (!this._lastQueryContext) {
+      Cu.reportError("Cannot delete - the latest query is not present");
+      return false;
+    }
+
+    const selectedResult = this.input.view.selectedResult;
+    if (!selectedResult ||
+        selectedResult.source != UrlbarUtils.MATCH_SOURCE.HISTORY) {
+      return false;
+    }
+
+    let index = this._lastQueryContext.results.indexOf(selectedResult);
+    if (!index) {
+      Cu.reportError("Failed to find the selected result in the results");
+      return false;
+    }
+
+    this._lastQueryContext.results.splice(index, 1);
+    this._notify("onQueryResultRemoved", index);
+
+    PlacesUtils.history.remove(selectedResult.payload.url).catch(Cu.reportError);
+    return true;
+  }
+
+  /**
    * Internal function to notify listeners of results.
    *
    * @param {string} name Name of the notification.
    * @param {object} params Parameters to pass with the notification.
    */
   _notify(name, ...params) {
     for (let listener of this._listeners) {
       try {
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -128,16 +128,51 @@ class UrlbarView {
     this._rows.textContent = "";
     this._queryContext = queryContext;
     for (let resultIndex in queryContext.results) {
       this._addRow(resultIndex);
     }
     this._openPanel();
   }
 
+  /**
+   * Handles removing a result from the view when it is removed from the query,
+   * and attempts to select the new result on the same row.
+   *
+   * This assumes that the result rows are in index order.
+   *
+   * @param {number} index The index of the result that has been removed.
+   */
+  onQueryResultRemoved(index) {
+    // Change the index for any rows above the removed index.
+    for (let i = index + 1; i < this._rows.children.length; i++) {
+      let child = this._rows.children[i];
+      child.setAttribute("resultIndex", child.getAttribute("resultIndex") - 1);
+    }
+
+    let rowToRemove = this._rows.children[index];
+    rowToRemove.remove();
+
+    if (rowToRemove != this._selected) {
+      return;
+    }
+
+    // Select the row at the same index, if possible.
+    let newSelectionIndex = index;
+    if (index >= this._queryContext.results.length) {
+      newSelectionIndex = this._queryContext.results.length - 1;
+    }
+    if (newSelectionIndex >= 0) {
+      this._selected = this._rows.children[newSelectionIndex];
+      this._selected.setAttribute("selected", true);
+    }
+
+    this.input.setValueFromResult(this._queryContext.results[newSelectionIndex]);
+  }
+
   // Private methods below.
 
   _getBoundsWithoutFlushing(element) {
     return this.window.windowUtils.getBoundsWithoutFlushing(element);
   }
 
   _createElement(name) {
     return this.document.createElementNS("http://www.w3.org/1999/xhtml", name);