Bug 1525910 - QuantumBar: Pressing the down key should first go to the end of line, then open the results. r=Standard8
authorDão Gottwald <dao@mozilla.com>
Tue, 05 Mar 2019 15:16:00 +0000
changeset 520262 2ee118b40f31cc0b201e26a32135581b33329697
parent 520261 c76f213ca5bd7db75e82d490eba53ef7427a750c
child 520263 ad6491a3248159b88e915c13de7e70af8babd568
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1525910
milestone67.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 1525910 - QuantumBar: Pressing the down key should first go to the end of line, then open the results. r=Standard8 Differential Revision: https://phabricator.services.mozilla.com/D20437
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarEventBufferer.jsm
browser/components/urlbar/tests/UrlbarTestUtils.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_caret_navigation.js
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -188,16 +188,52 @@ class UrlbarController {
    * or save resourced on repeated searches.
    */
   viewContextChanged() {
     this.cancelQuery();
     this._notify("onViewContextChanged");
   }
 
   /**
+   * Checks whether a keyboard event that would normally open the view should
+   * instead be handled natively by the input field.
+   * On certain platforms, the up and down keys can be used to move the caret,
+   * in which case we only want to open the view if the caret is at the
+   * start or end of the input.
+   *
+   * @param {KeyboardEvent} event
+   *   The DOM KeyboardEvent.
+   * @returns {boolean}
+   *   Returns true if the event should move the caret instead of opening the
+   *   view.
+   */
+  keyEventMovesCaret(event) {
+    if (this.view.isOpen) {
+      return false;
+    }
+    if (AppConstants.platform != "macosx" &&
+        AppConstants.platform != "linux") {
+      return false;
+    }
+    let isArrowUp = event.keyCode == KeyEvent.DOM_VK_UP;
+    let isArrowDown = event.keyCode == KeyEvent.DOM_VK_DOWN;
+    if (!isArrowUp && !isArrowDown) {
+      return false;
+    }
+    let start = this.input.selectionStart;
+    let end = this.input.selectionEnd;
+    if (end != start ||
+        (isArrowUp && start > 0) ||
+        (isArrowDown && end < this.input.textValue.length)) {
+      return true;
+    }
+    return false;
+  }
+
+  /**
    * 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";
@@ -256,16 +292,19 @@ class UrlbarController {
           this.userSelectionBehavior = "arrow";
           this.view.selectBy(
             event.keyCode == KeyEvent.DOM_VK_PAGE_DOWN ||
             event.keyCode == KeyEvent.DOM_VK_PAGE_UP ?
               5 : 1,
             { reverse: event.keyCode == KeyEvent.DOM_VK_UP ||
                        event.keyCode == KeyEvent.DOM_VK_PAGE_UP });
         } else {
+          if (this.keyEventMovesCaret(event)) {
+            break;
+          }
           this.input.startQuery();
         }
         event.preventDefault();
         break;
       case KeyEvent.DOM_VK_DELETE:
       case KeyEvent.DOM_VK_BACK_SPACE:
         if (event.shiftKey && this.view.isOpen && this._handleDeleteEntry()) {
           event.preventDefault();
--- a/browser/components/urlbar/UrlbarEventBufferer.jsm
+++ b/browser/components/urlbar/UrlbarEventBufferer.jsm
@@ -237,16 +237,21 @@ class UrlbarEventBufferer {
     let isMacNavigation = AppConstants.platform == "macosx" &&
                           event.ctrlKey &&
                           this.input.view.isOpen &&
                           (event.key === "n" || event.key === "p");
     if (!DEFERRED_KEY_CODES.has(event.keyCode) && !isMacNavigation) {
       return false;
     }
 
+    if (DEFERRED_KEY_CODES.has(event.keyCode) &&
+        this.input.controller.keyEventMovesCaret(event)) {
+      return false;
+    }
+
     // This is an event that we'd defer, but if enough time has passed since the
     // start of the search, we don't want to block the user's workflow anymore.
     if (this._lastQuery.startDate + DEFERRING_TIMEOUT_MS <= Cu.now()) {
       return false;
     }
 
     if (event.keyCode == KeyEvent.DOM_VK_TAB && !this.input.view.isOpen) {
       // The view is closed and the user pressed the Tab key.  The focus should
--- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm
+++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm
@@ -191,16 +191,25 @@ var UrlbarTestUtils = {
    * @param {function} [closeFn] Function to be used to close the popup, if not
    *        supplied it will default to a closing the popup directly.
    * @returns {Promise} resolved once the popup is closed
    */
   promisePopupClose(win, closeFn = null) {
     let urlbar = getUrlbarAbstraction(win);
     return urlbar.promisePopupClose(closeFn);
   },
+
+  /**
+   * @param {object} win The browser window
+   * @returns {boolean} Whether the popup is open
+   */
+  isPopupOpen(win) {
+    let urlbar = getUrlbarAbstraction(win);
+    return urlbar.isPopupOpen();
+  },
 };
 
 /**
  * Maps windows to urlbar abstractions.
  */
 var gUrlbarAbstractions = new WeakMap();
 
 function getUrlbarAbstraction(win) {
@@ -473,14 +482,15 @@ class UrlbarAbstraction {
   }
 
   async promisePopupClose(closeFn) {
     if (closeFn) {
       await closeFn();
     } else {
       this.closePopup();
     }
-    if (!this.quantumbar) {
-      return BrowserTestUtils.waitForPopupEvent(this.urlbar.popup, "hidden");
-    }
-    return BrowserTestUtils.waitForPopupEvent(this.urlbar.view.panel, "hidden");
+    return BrowserTestUtils.waitForPopupEvent(this.panel, "hidden");
+  }
+
+  isPopupOpen() {
+    return this.panel.state == "open" || this.panel.state == "showing";
   }
 }
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -27,16 +27,17 @@ skip-if = os != "mac" # Mac only feature
 skip-if = true # Bug 1531348 - Failing with QuantumBar.
 [browser_autoFill_canonize.js]
 skip-if = true # Bug 1531348 - Failing with QuantumBar.
 [browser_autoFill_preserveCase.js]
 skip-if = true # Bug 1531348 - Failing with QuantumBar.
 [browser_autoFill_trimURLs.js]
 skip-if = true # Bug 1531348 - Failing with QuantumBar.
 [browser_canonizeURL.js]
+[browser_caret_navigation.js]
 [browser_dragdropURL.js]
 [browser_keepStateAcrossTabSwitches.js]
 [browser_keyword_override.js]
 [browser_keyword_select_and_type.js]
 [browser_keyword.js]
 support-files =
   print_postdata.sjs
 [browser_locationBarCommand.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_caret_navigation.js
@@ -0,0 +1,71 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Tests that up and down keys move the caret on certain platforms, and that
+ * opening the popup doesn't change the caret position.
+ */
+
+add_task(async function() {
+  await promiseAutocompleteResultPopup("This is a generic sentence");
+  await UrlbarTestUtils.promisePopupClose(window);
+
+  const INITIAL_SELECTION_START = 3;
+  const INITIAL_SELECTION_END = 10;
+  gURLBar.selectionStart = INITIAL_SELECTION_START;
+  gURLBar.selectionEnd = INITIAL_SELECTION_END;
+
+  if (AppConstants.platform == "macosx" ||
+      AppConstants.platform == "linux") {
+    if (AppConstants.platform == "linux") {
+      await checkCaretMoves("KEY_ArrowUp", INITIAL_SELECTION_START, "Selection should be collapsed to its start");
+
+      gURLBar.selectionStart = INITIAL_SELECTION_START;
+      gURLBar.selectionEnd = INITIAL_SELECTION_END;
+      await checkCaretMoves("KEY_ArrowDown", INITIAL_SELECTION_END, "Selection should be collapsed to its end");
+    }
+
+    await checkCaretMoves("KEY_ArrowDown", gURLBar.textValue.length, "Caret should have moved to the end");
+    await checkPopupOpens("KEY_ArrowDown", gURLBar.textValue.length);
+
+    await checkCaretMoves("KEY_ArrowUp", 0, "Caret should have moved to the start");
+    await checkPopupOpens("KEY_ArrowUp", 0);
+  } else {
+    await checkPopupOpens("KEY_ArrowDown", gURLBar.textValue.length);
+    await checkPopupOpens("KEY_ArrowUp", gURLBar.textValue.length);
+  }
+});
+
+async function checkCaretMoves(key, pos, msg) {
+  checkIfKeyStartsQuery(key, false);
+  Assert.equal(UrlbarTestUtils.isPopupOpen(window), false, `${key}: Popup shouldn't be open`);
+  Assert.equal(gURLBar.selectionStart, gURLBar.selectionEnd, `${key}: Input selection should be empty`);
+  Assert.equal(gURLBar.selectionStart, pos, `${key}: ${msg}`);
+}
+
+async function checkPopupOpens(key, expectedCaretPosition) {
+  await UrlbarTestUtils.promisePopupOpen(window, () => {
+    checkIfKeyStartsQuery(key, true);
+  });
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), 0, `${key}: Heuristic result should be selected`);
+  Assert.equal(gURLBar.selectionStart, gURLBar.selectionEnd, `${key}: Input selection should be empty`);
+  Assert.equal(gURLBar.selectionStart, expectedCaretPosition, `${key}: Caret is at the expected position`);
+  await UrlbarTestUtils.promisePopupClose(window);
+}
+
+function checkIfKeyStartsQuery(key, shouldStartQuery) {
+  let queryStarted = false;
+  let queryListener = {
+    onQueryStarted() {
+      queryStarted = true;
+    },
+  };
+  gURLBar.controller.addQueryListener(queryListener);
+  EventUtils.synthesizeKey(key);
+  gURLBar.eventBufferer.replayAllDeferredEvents();
+  gURLBar.controller.removeQueryListener(queryListener);
+  Assert.equal(queryStarted, shouldStartQuery,
+               `${key}: Should${shouldStartQuery ? "" : "n't"} have started a query`);
+}