Bug 1492842 - Implement UrlbarInput up/down key event handling to navigate within the UrlbarView. r=Standard8
authorDão Gottwald <dao@mozilla.com>
Thu, 06 Dec 2018 21:33:30 +0000
changeset 508784 8a74a0cfd788e50d2bff25844644c970d1cb5437
parent 508783 7c02d6c28133994b30e2dd8ee3ae43e3d0cbd23f
child 508785 a9c528fe96d5df7ee16d1c9d8f84c03b04ef850b
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1492842
milestone65.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 1492842 - Implement UrlbarInput up/down key event handling to navigate within the UrlbarView. r=Standard8 Differential Revision: https://phabricator.services.mozilla.com/D13794
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarView.jsm
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -103,16 +103,36 @@ class UrlbarController {
 
     this.manager = options.manager || UrlbarProvidersManager;
     this.browserWindow = options.browserWindow;
 
     this._listeners = new Set();
   }
 
   /**
+   * Hooks up the controller with an input.
+   *
+   * @param {UrlbarInput} input
+   *   The UrlbarInput instance associated with this controller.
+   */
+  setInput(input) {
+    this.input = input;
+  }
+
+  /**
+   * Hooks up the controller with a view.
+   *
+   * @param {UrlbarView} view
+   *   The UrlbarView instance associated with this controller.
+   */
+  setView(view) {
+    this.view = view;
+  }
+
+  /**
    * Takes a query context and starts the query based on the user input.
    *
    * @param {QueryContext} queryContext The query details.
    */
   async startQuery(queryContext) {
     queryContext.autoFill = UrlbarPrefs.get("autoFill");
 
     this._notify("onQueryStarted", queryContext);
@@ -169,16 +189,65 @@ class UrlbarController {
    * When switching tabs, clear some internal caches to handle cases like
    * backspace, autofill or repeated searches.
    */
   tabContextChanged() {
     // TODO: implementation needed (bug 1496685)
   }
 
   /**
+   * 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) {
+    // Handle readline/emacs-style navigation bindings on Mac.
+    if (AppConstants.platform == "macosx" &&
+        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_RETURN:
+        if (AppConstants.platform == "macosx" &&
+            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);
+        return;
+      case KeyEvent.DOM_VK_TAB:
+        this.view.selectNextItem({ reverse: event.shiftKey });
+        event.preventDefault();
+        break;
+      case KeyEvent.DOM_VK_DOWN:
+        if (!event.ctrlKey && !event.altKey) {
+          this.view.selectNextItem();
+          event.preventDefault();
+        }
+        break;
+      case KeyEvent.DOM_VK_UP:
+        if (!event.ctrlKey && !event.altKey) {
+          this.view.selectNextItem({ reverse: true });
+          event.preventDefault();
+        }
+        break;
+    }
+  }
+
+  /**
    * 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/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -45,16 +45,17 @@ class UrlbarInput {
     this.textbox.clickSelectsAll = UrlbarPrefs.get("clickSelectsAll");
 
     this.panel = options.panel;
     this.window = this.textbox.ownerGlobal;
     this.document = this.window.document;
     this.controller = options.controller || new UrlbarController({
       browserWindow: this.window,
     });
+    this.controller.setInput(this);
     this.view = new UrlbarView(this);
     this.valueIsTyped = false;
     this.userInitiatedFocus = false;
     this.isPrivate = PrivateBrowsingUtils.isWindowPrivate(this.window);
     this._untrimmedValue = "";
 
     // Forward textbox methods and properties.
     const METHODS = ["addEventListener", "removeEventListener",
@@ -99,17 +100,17 @@ class UrlbarInput {
     this.inputField.addEventListener("blur", this);
     this.inputField.addEventListener("focus", this);
     this.inputField.addEventListener("mousedown", this);
     this.inputField.addEventListener("mouseover", this);
     this.inputField.addEventListener("overflow", this);
     this.inputField.addEventListener("underflow", this);
     this.inputField.addEventListener("scrollend", this);
     this.inputField.addEventListener("select", this);
-    this.inputField.addEventListener("keyup", this);
+    this.inputField.addEventListener("keydown", this);
 
     this.inputField.controllers.insertControllerAt(0, new CopyCutController(this));
   }
 
   /**
    * Shortens the given value, usually by removing http:// and trailing slashes,
    * such that calling nsIURIFixup::createFixupURI with the result will produce
    * the same URI.
@@ -175,17 +176,17 @@ class UrlbarInput {
       throw new Error("Unrecognized UrlbarInput event: " + event.type);
     }
   }
 
   /**
    * Handles an event which would cause a url or text to be opened.
    * XXX the name is currently handleCommand which is compatible with
    * urlbarBindings. However, it is no longer called automatically by autocomplete,
-   * See _on_keyup.
+   * See _on_keydown.
    *
    * @param {Event} event The event triggering the open.
    * @param {string} [openWhere] Where we expect the result to be opened.
    * @param {object} [openParams]
    *   The parameters related to where the result will be opened.
    * @param {object} [triggeringPrincipal]
    *   The principal that the action was triggered from.
    */
@@ -250,22 +251,22 @@ class UrlbarInput {
     }
 
     this._loadURL(url, where, openParams);
 
     this.view.close();
   }
 
   /**
-   * Called by the view when a result is selected.
+   * Called by the view when a result is picked.
    *
-   * @param {Event} event The event that selected the result.
-   * @param {UrlbarMatch} result The result that was selected.
+   * @param {Event} event The event that picked the result.
+   * @param {UrlbarMatch} result The result that was picked.
    */
-  resultSelected(event, result) {
+  pickResult(event, result) {
     this.setValueFromResult(result);
 
     // TODO: Work out how we get the user selection behavior, probably via passing
     // it in, since we don't have the old autocomplete controller to work with.
     // BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
     //   event, this.userSelectionBehavior);
 
     let where = this._whereToOpen(event);
@@ -707,23 +708,18 @@ class UrlbarInput {
   _on_scrollend(event) {
     this._updateTextOverflow();
   }
 
   _on_TabSelect(event) {
     this.controller.tabContextChanged();
   }
 
-  _on_keyup(event) {
-    // 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.
-    if (event.key == "Enter") {
-      this.handleCommand(event);
-    }
+  _on_keydown(event) {
+    this.controller.handleKeyNavigation(event);
   }
 }
 
 /**
  * Handles copy and cut commands for the urlbar.
  */
 class CopyCutController {
   /**
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -11,58 +11,106 @@ XPCOMUtils.defineLazyModuleGetters(this,
   UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
 /**
  * Receives and displays address bar autocomplete results.
  */
 class UrlbarView {
   /**
-   * @param {UrlbarInput} urlbar
+   * @param {UrlbarInput} input
    *   The UrlbarInput instance belonging to this UrlbarView instance.
    */
-  constructor(urlbar) {
-    this.urlbar = urlbar;
-    this.panel = urlbar.panel;
-    this.controller = urlbar.controller;
-    this.document = urlbar.panel.ownerDocument;
+  constructor(input) {
+    this.input = input;
+    this.panel = input.panel;
+    this.controller = input.controller;
+    this.document = this.panel.ownerDocument;
     this.window = this.document.defaultView;
 
     this._mainContainer = this.panel.querySelector(".urlbarView-body-inner");
     this._rows = this.panel.querySelector(".urlbarView-results");
 
+    this._rows.addEventListener("click", this);
+
     // For the horizontal fade-out effect, set the overflow attribute on result
     // rows when they overflow.
     this._rows.addEventListener("overflow", this);
     this._rows.addEventListener("underflow", this);
 
+    this.controller.setView(this);
     this.controller.addQueryListener(this);
   }
 
   get oneOffSearchButtons() {
     return this._oneOffSearchButtons ||
       (this._oneOffSearchButtons =
          new this.window.SearchOneOffs(this.panel.querySelector(".search-one-offs")));
   }
 
   /**
+   * @returns {boolean}
+   *   Whether the panel is open.
+   */
+  get isOpen() {
+    return this.panel.state == "open" || this.panel.state == "showing";
+  }
+
+  /**
+   * Selects the next or previous view item. An item could be an autocomplete
+   * result or a one-off search button.
+   *
+   * @param {boolean} options.reverse
+   *   Set to true to select the previous item. By default the next item
+   *   will be selected.
+   */
+  selectNextItem({reverse = false} = {}) {
+    if (!this.isOpen) {
+      this.open();
+      return;
+    }
+
+    // TODO: handle one-off search buttons
+
+    let row;
+    if (reverse) {
+      row = this._selected.previousElementSibling ||
+            this._rows.lastElementChild;
+    } else {
+      row = this._selected.nextElementSibling ||
+            this._rows.firstElementChild;
+    }
+
+    this._selected.toggleAttribute("selected", false);
+    this._selected = row;
+    row.toggleAttribute("selected", true);
+
+    let resultIndex = row.getAttribute("resultIndex");
+    let result = this._queryContext.results[resultIndex];
+    if (result) {
+      this.input.setValueFromResult(result);
+    }
+  }
+
+  /**
    * Opens the autocomplete results popup.
    */
   open() {
     this.panel.removeAttribute("hidden");
 
     this._alignPanel();
 
     // TODO: Search one off buttons are a stub right now.
     //       We'll need to set them up properly.
     this.oneOffSearchButtons;
 
-    this.panel.openPopup(this.urlbar.textbox.closest("toolbar"), "after_end", 0, -1);
+    this.panel.openPopup(this.input.textbox.closest("toolbar"), "after_end", 0, -1);
 
-    this._rows.firstElementChild.toggleAttribute("selected", true);
+    this._selected = this._rows.firstElementChild;
+    this._selected.toggleAttribute("selected", true);
   }
 
   /**
    * Closes the autocomplete results popup.
    */
   close() {
     this.panel.hidePopup();
   }
@@ -106,22 +154,22 @@ class UrlbarView {
     let documentRect =
       this._getBoundsWithoutFlushing(this.document.documentElement);
     let width = documentRect.right - documentRect.left;
     this.panel.setAttribute("width", width);
 
     // Subtract two pixels for left and right borders on the panel.
     this._mainContainer.style.maxWidth = (width - 2) + "px";
 
-    // Keep the popup items' site icons aligned with the urlbar's identity
+    // Keep the popup items' site icons aligned with the input's identity
     // icon if it's not too far from the edge of the window.  We define
     // "too far" as "more than 30% of the window's width AND more than
     // 250px".
     let boundToCheck = this.window.RTL_UI ? "right" : "left";
-    let inputRect = this._getBoundsWithoutFlushing(this.urlbar.textbox);
+    let inputRect = this._getBoundsWithoutFlushing(this.input.textbox);
     let startOffset = Math.abs(inputRect[boundToCheck] - documentRect[boundToCheck]);
     let alignSiteIcons = startOffset / width <= 0.3 || startOffset <= 250;
     if (alignSiteIcons) {
       // Calculate the end margin if we have a start margin.
       let boundToCheckEnd = this.window.RTL_UI ? "left" : "right";
       let endOffset = Math.abs(inputRect[boundToCheckEnd] -
                                documentRect[boundToCheckEnd]);
       if (endOffset > startOffset * 2) {
@@ -143,17 +191,16 @@ class UrlbarView {
       this.panel.style.removeProperty("--item-padding-end");
     }
   }
 
   _addRow(resultIndex) {
     let result = this._queryContext.results[resultIndex];
     let item = this._createElement("div");
     item.className = "urlbarView-row";
-    item.addEventListener("click", this);
     item.setAttribute("resultIndex", resultIndex);
     if (result.type == UrlbarUtils.MATCH_TYPE.TAB_SWITCH) {
       item.setAttribute("action", "switch-to-tab");
     }
 
     let content = this._createElement("span");
     content.className = "urlbarView-row-inner";
     item.appendChild(content);
@@ -203,17 +250,17 @@ class UrlbarView {
   _on_click(event) {
     let row = event.target;
     while (!row.classList.contains("urlbarView-row")) {
       row = row.parentNode;
     }
     let resultIndex = row.getAttribute("resultIndex");
     let result = this._queryContext.results[resultIndex];
     if (result) {
-      this.urlbar.resultSelected(event, result);
+      this.input.pickResult(event, result);
     }
     this.close();
   }
 
   _on_overflow(event) {
     if (event.target.classList.contains("urlbarView-row-inner")) {
       event.target.toggleAttribute("overflow", true);
     }