Bug 1540710 - Down arrow key on the current URL/text should search for the URL/text, and clicking on the history dropmarker should keep the input's current value. r=dao
authorDrew Willcoxon <adw@mozilla.com>
Thu, 11 Apr 2019 21:09:50 +0000
changeset 469085 38111dd15e5918deb47a449e4e4524d78b33424e
parent 469084 0078d5355d9a2dc39e3933a9cd0287ffd638e034
child 469086 180292b783d8528745c230035304b738471abb9c
push id35856
push usercsabou@mozilla.com
push dateFri, 12 Apr 2019 03:19:48 +0000
treeherdermozilla-central@940684cd1065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1540710
milestone68.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 1540710 - Down arrow key on the current URL/text should search for the URL/text, and clicking on the history dropmarker should keep the input's current value. r=dao Differential Revision: https://phabricator.services.mozilla.com/D25906
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_downArrowKeySearch.js
browser/components/urlbar/tests/browser/browser_dropmarker.js
browser/components/urlbar/tests/browser/browser_restoreEmptyInput.js
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -312,17 +312,17 @@ class UrlbarController {
               { reverse: event.keyCode == KeyEvent.DOM_VK_UP ||
                         event.keyCode == KeyEvent.DOM_VK_PAGE_UP });
           }
         } else {
           if (this.keyEventMovesCaret(event)) {
             break;
           }
           if (executeAction) {
-            this.input.startQuery();
+            this.input.startQuery({ searchString: this.input.textValue });
           }
         }
         event.preventDefault();
         break;
       case KeyEvent.DOM_VK_LEFT:
       case KeyEvent.DOM_VK_RIGHT:
       case KeyEvent.DOM_VK_HOME:
       case KeyEvent.DOM_VK_END:
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -514,50 +514,54 @@ class UrlbarInput {
    * Starts a query based on the current input value.
    *
    * @param {boolean} [options.allowAutofill]
    *   Whether or not to allow providers to include autofill results.
    * @param {number} [options.lastKey]
    *   The last key the user entered (as a key code).
    * @param {string} [options.searchString]
    *   The search string.  If not given, the current input value is used.
-   *   Otherwise, the current input value must start with this value.  The
-   *   intended use for this parameter is related to the autofill placeholder.
-   *   When the placeholder is autofilled before the new search starts, the
-   *   current input value will be the entire autofilled placeholder, not the
-   *   value the user typed, which is the value we should search with.
-   * @param {boolean} [resetSearchState]
+   *   Otherwise, the current input value must start with this value.
+   * @param {boolean} [options.resetSearchState]
    *   If this is the first search of a user interaction with the input, set
    *   this to true (the default) so that search-related state from the previous
    *   interaction doesn't interfere with the new interaction.  Otherwise set it
    *   to false so that state is maintained during a single interaction.  The
    *   intended use for this parameter is that it should be set to false when
    *   this method is called due to input events.
+   * @param {boolean} [options.allowEmptyInput]
+   *   If true and the search string is empty, then the input will become empty
+   *   when no result is selected.  If false, the input will continue showing
+   *   the last non-empty search string when no result is selected.
    */
   startQuery({
     allowAutofill = true,
     lastKey = null,
     searchString = null,
     resetSearchState = true,
+    allowEmptyInput = true,
   } = {}) {
     if (this._suppressStartQuery) {
       return;
     }
 
     if (resetSearchState) {
       this._resetSearchState();
     }
 
     if (!searchString) {
       searchString = (this.getAttribute("pageproxystate") == "valid") ?
                      "" : this.textValue;
     } else if (!this.textValue.startsWith(searchString)) {
       throw new Error("The current value doesn't start with the search string");
     }
-    this._lastSearchString = searchString;
+
+    if (searchString || allowEmptyInput) {
+      this._lastSearchString = searchString;
+    }
 
     // TODO (Bug 1522902): This promise is necessary for tests, because some
     // tests are not listening for completion when starting a query through
     // other methods than startQuery (input events for example).
     this.lastQueryContextPromise = this.controller.startQuery(new UrlbarQueryContext({
       allowAutofill,
       isPrivate: this.isPrivate,
       lastKey,
@@ -1206,17 +1210,17 @@ class UrlbarInput {
       return;
     }
 
     if (event.originalTarget.classList.contains("urlbar-history-dropmarker") &&
         event.button == 0) {
       if (this.view.isOpen) {
         this.view.close();
       } else {
-        this.startQuery();
+        this.startQuery({ allowEmptyInput: false });
       }
     }
   }
 
   _on_input() {
     let value = this.textValue;
     this.valueIsTyped = true;
     this._untrimmedValue = value;
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -30,16 +30,17 @@ skip-if = os != "mac" # Mac only feature
 [browser_autoFill_preserve.js]
 [browser_autoFill_trimURLs.js]
 [browser_autoFill_typed.js]
 [browser_canonizeURL.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_caret_navigation.js]
+[browser_downArrowKeySearch.js]
 [browser_dragdropURL.js]
 [browser_dropmarker.js]
 [browser_ime_composition.js]
 [browser_keepStateAcrossTabSwitches.js]
 [browser_keywordBookmarklets.js]
 [browser_keyword_override.js]
 [browser_keywordSearch.js]
 [browser_keywordSearch_postData.js]
@@ -63,16 +64,17 @@ subsuite = clipboard
 [browser_privateBrowsingWindowChange.js]
 [browser_raceWithTabs.js]
 skip-if = os == "linux" # Bug 1533807
 [browser_redirect_error.js]
 support-files = redirect_error.sjs
 [browser_remotetab.js]
 [browser_removeUnsafeProtocolsFromURLBarPaste.js]
 subsuite = clipboard
+[browser_restoreEmptyInput.js]
 [browser_search_favicon.js]
 skip-if = true # Bug 1526222 - Doesn't currently work with QuantumBar
 [browser_searchTelemetry.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_selectionKeyNavigation.js]
 [browser_stop_pending.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_downArrowKeySearch.js
@@ -0,0 +1,56 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Checks that pressing the down arrow key starts the proper searches, depending
+// on the input value/state.
+
+"use strict";
+
+add_task(async function init() {
+  await PlacesUtils.history.clear();
+  await PlacesTestUtils.addVisits("http://example.com/");
+  registerCleanupFunction(async function() {
+    await PlacesUtils.history.clear();
+  });
+});
+
+add_task(async function url() {
+  await BrowserTestUtils.withNewTab("http://example.com/", async () => {
+    gURLBar.focus();
+    gURLBar.selectionEnd = gURLBar.value.length;
+    gURLBar.selectionStart = gURLBar.value.length;
+    EventUtils.synthesizeKey("KEY_ArrowDown");
+    await UrlbarTestUtils.promiseSearchComplete(window);
+    Assert.equal(UrlbarTestUtils.getSelectedIndex(window), 0);
+    let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+    Assert.ok(details.autofill);
+    Assert.equal(details.url, "http://example.com/");
+    Assert.equal(gURLBar.textValue, "example.com/");
+    await UrlbarTestUtils.promisePopupClose(window);
+  });
+});
+
+add_task(async function userTyping() {
+  await promiseAutocompleteResultPopup("foo", window, true);
+  await UrlbarTestUtils.promisePopupClose(window);
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  await UrlbarTestUtils.promiseSearchComplete(window);
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), 0);
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(details.type, UrlbarUtils.RESULT_TYPE.SEARCH);
+  Assert.ok(details.searchParams);
+  Assert.equal(details.searchParams.query, "foo");
+  Assert.equal(gURLBar.textValue, "foo");
+  await UrlbarTestUtils.promisePopupClose(window);
+});
+
+add_task(async function empty() {
+  await promiseAutocompleteResultPopup("", window, true);
+  await UrlbarTestUtils.promisePopupClose(window);
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  await UrlbarTestUtils.promiseSearchComplete(window);
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), -1);
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(details.url, "http://example.com/");
+  Assert.equal(gURLBar.textValue, "");
+});
--- a/browser/components/urlbar/tests/browser/browser_dropmarker.js
+++ b/browser/components/urlbar/tests/browser/browser_dropmarker.js
@@ -9,11 +9,13 @@ add_task(async function() {
     await UrlbarTestUtils.promisePopupOpen(window, () => {
       let historyDropMarker =
         window.document.getAnonymousElementByAttribute(gURLBar.textbox, "anonid", "historydropmarker");
       EventUtils.synthesizeMouseAtCenter(historyDropMarker, {}, window);
     });
     let queryContext = await gURLBar.lastQueryContextPromise;
     is(queryContext.searchString, "",
        "Clicking the history dropmarker should initiate an empty search instead of searching for the loaded URL");
+    is(gURLBar.value, "example.com",
+       "Clicking the history dropmarker should not change the input value");
     await UrlbarTestUtils.promisePopupClose(window);
   });
 });
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_restoreEmptyInput.js
@@ -0,0 +1,34 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// When the input is empty and the view is opened, keying down through the
+// results and then out of the results should restore the empty input.
+
+"use strict";
+
+add_task(async function test() {
+  await PlacesTestUtils.addVisits("http://example.com/");
+  registerCleanupFunction(async function() {
+    await PlacesUtils.history.clear();
+  });
+
+  await promiseAutocompleteResultPopup("", window, true);
+
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), -1,
+               "Nothing selected");
+
+  let resultCount = UrlbarTestUtils.getResultCount(window);
+  Assert.ok(resultCount > 0, "At least one result");
+
+  for (let i = 0; i < resultCount; i++) {
+    EventUtils.synthesizeKey("KEY_ArrowDown");
+  }
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), resultCount - 1,
+               "Last result selected");
+  Assert.notEqual(gURLBar.value, "", "Input should not be empty");
+
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  Assert.equal(UrlbarTestUtils.getSelectedIndex(window), -1,
+               "Nothing selected");
+  Assert.equal(gURLBar.value, "", "Input should be empty");
+});