Bug 1506126 - Support clearing history for selected entries. r=dao

Differential Revision: https://phabricator.services.mozilla.com/D16332

--HG--
extra : moz-landing-system : lando

browser/base/content/test/urlbar/browser_urlbar_remove_match.js

@@ -1,4 +1,3 @@
-/* eslint-disable mozilla/no-arbitrary-setTimeout */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
@@ -29,3 +28,37 @@ add_task(async function test_remove_history() {
   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.");
+});

browser/components/urlbar/UrlbarController.jsm

@@ -10,8 +10,10 @@ 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";
@@ -28,6 +30,7 @@ const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
  * - onQueryResults(queryContext)
  * - onQueryCancelled(queryContext)
  * - onQueryFinished(queryContext)
+ * - onQueryResultRemoved(index)
  */
 class UrlbarController {
   /**
@@ -175,8 +178,9 @@ class UrlbarController {
    *   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")) {
@@ -191,7 +195,7 @@ class UrlbarController {
         event.preventDefault();
         break;
       case KeyEvent.DOM_VK_RETURN:
-        if (AppConstants.platform == "macosx" &&
+        if (isMac &&
             event.metaKey) {
           // Prevent beep on Mac.
           event.preventDefault();
@@ -219,7 +223,52 @@ class UrlbarController {
           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;
   }
 
   /**

browser/components/urlbar/UrlbarView.jsm

@@ -133,6 +133,41 @@ class UrlbarView {
     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) {