From cadc97f7fd81ff9c5876bf3302f8b5ea536f3005 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A3o=20Gottwald?= <dao@mozilla.com>
Date: Tue, 11 Dec 2018 15:14:56 +0000
Subject: [PATCH] Bug 1512661 - Let up/down keys open the results popup.
 r=Standard8

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

--HG--
extra : moz-landing-system : lando
---
 .../components/urlbar/UrlbarController.jsm    | 18 +++++----
 browser/components/urlbar/UrlbarInput.jsm     | 32 +++++++++++-----
 browser/components/urlbar/UrlbarView.jsm      | 38 +++++++++----------
 3 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/browser/components/urlbar/UrlbarController.jsm b/browser/components/urlbar/UrlbarController.jsm
index a1360f30a6aa..70b9d435764d 100644
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -37,7 +37,7 @@ class QueryContext {
    *   The maximum number of results that will be displayed for this query.
    * @param {boolean} [options.autoFill]
    *   Whether or not to include autofill results. Optional, as this is normally
-   *   set by the AddressBarController.
+   *   set by the UrlbarController.
    */
   constructor(options = {}) {
     this._checkRequiredOptions(options, [
@@ -224,18 +224,20 @@ class UrlbarController {
         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();
+        if (this.view.isOpen) {
+          this.view.selectNextItem({ reverse: event.shiftKey });
           event.preventDefault();
         }
         break;
+      case KeyEvent.DOM_VK_DOWN:
       case KeyEvent.DOM_VK_UP:
         if (!event.ctrlKey && !event.altKey) {
-          this.view.selectNextItem({ reverse: true });
+          if (this.view.isOpen) {
+            this.view.selectNextItem({
+              reverse: event.keyCode == KeyEvent.DOM_VK_UP });
+          } else {
+            this.input.startQuery();
+          }
           event.preventDefault();
         }
         break;
diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm
index 9a23c7e2a64a..afc88cc65ba8 100644
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -135,10 +135,6 @@ class UrlbarInput {
     this.view.close();
   }
 
-  openResults() {
-    this.view.open();
-  }
-
   /**
    * Converts an internal URI (e.g. a wyciwyg URI) into one which we can
    * expose to the user.
@@ -331,6 +327,26 @@ class UrlbarInput {
     this.value = val;
   }
 
+  /**
+   * Starts a query based on the user input.
+   *
+   * @param {string} [options.searchString]
+   *   The string the user entered in autocomplete.
+   * @param {number} [options.lastKey]
+   *   The last key the user entered (as a key code).
+   */
+  startQuery({
+    searchString = "",
+    lastKey = null,
+  } = {}) {
+    this.controller.startQuery(new QueryContext({
+      searchString,
+      lastKey,
+      maxResults: UrlbarPrefs.get("maxRichResults"),
+      isPrivate: this.isPrivate,
+    }));
+  }
+
   // Getters and Setters below.
 
   get focused() {
@@ -653,12 +669,10 @@ class UrlbarInput {
     }
 
     // XXX Fill in lastKey, and add anything else we need.
-    this.controller.startQuery(new QueryContext({
+    this.startQuery({
       searchString: value,
-      lastKey: "",
-      maxResults: UrlbarPrefs.get("maxRichResults"),
-      isPrivate: this.isPrivate,
-    }));
+      lastKey: null,
+    });
   }
 
   _on_select(event) {
diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm
index 7d2bfa89a3b6..23f88bb29d02 100644
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -64,8 +64,7 @@ class UrlbarView {
    */
   selectNextItem({reverse = false} = {}) {
     if (!this.isOpen) {
-      this.open();
-      return;
+      throw new Error("UrlbarView: Cannot select an item if the view isn't open.");
     }
 
     // TODO: handle one-off search buttons
@@ -90,24 +89,6 @@ class UrlbarView {
     }
   }
 
-  /**
-   * 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.input.textbox.closest("toolbar"), "after_end", 0, -1);
-
-    this._selected = this._rows.firstElementChild;
-    this._selected.toggleAttribute("selected", true);
-  }
-
   /**
    * Closes the autocomplete results popup.
    */
@@ -136,7 +117,7 @@ class UrlbarView {
     for (let resultIndex in queryContext.results) {
       this._addRow(resultIndex);
     }
-    this.open();
+    this._openPanel();
   }
 
   // Private methods below.
@@ -149,6 +130,21 @@ class UrlbarView {
     return this.document.createElementNS("http://www.w3.org/1999/xhtml", name);
   }
 
+  _openPanel() {
+    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.input.textbox.closest("toolbar"), "after_end", 0, -1);
+
+    this._selected = this._rows.firstElementChild;
+    this._selected.toggleAttribute("selected", true);
+  }
+
   _alignPanel() {
     // Make the panel span the width of the window.
     let documentRect =