Bug 1541929 - Don't autofill the first result in some cases. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Mon, 15 Apr 2019 13:15:30 +0000
changeset 469513 ea02b41a2b1d
parent 469512 e25e95c59e6b
child 469514 d525a800fe20
push id35873
push userccoroiu@mozilla.com
push dateMon, 15 Apr 2019 21:36:26 +0000
treeherdermozilla-central@b8f49a14c458 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1541929
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 1541929 - Don't autofill the first result in some cases. r=mak We need to handle autofilling the first result separately from autofilling results in general (which happens in UrlbarInput.setValueFromResult), so add a new UrlbarInput.autofillFirstResult method. The controller calls it instead of setValueFromResult. I ported the logic from nsAutoCompleteController, as described in the bug. Other changes are related to the new test for this. As part of this work, I was interested in learning how awesomebar handles browser_autoFill_typed.js, so I added it to the legacy tests, with a small tweak in the test for awesomebar. Differential Revision: https://phabricator.services.mozilla.com/D26852
accessible/tests/browser/general/browser_test_urlbar.js
browser/base/content/test/performance/head.js
browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js
browser/components/search/test/browser/browser_oneOffContextMenu_setDefault.js
browser/components/sessionstore/test/browser_tabs_in_urlbar.js
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/UrlbarTestUtils.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_autoFill_firstResult.js
browser/components/urlbar/tests/browser/browser_autoFill_typed.js
browser/components/urlbar/tests/browser/browser_autoFill_undo.js
browser/components/urlbar/tests/browser/browser_autocomplete_autoselect.js
browser/components/urlbar/tests/browser/browser_switchToTab_closes_newtab.js
browser/components/urlbar/tests/browser/browser_switchToTab_fullUrl_repeatedKeydown.js
browser/components/urlbar/tests/browser/head-common.js
browser/components/urlbar/tests/legacy/browser.ini
browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
browser/modules/test/browser/browser_UsageTelemetry_urlbar_extension.js
browser/modules/test/browser/browser_UsageTelemetry_urlbar_places.js
browser/modules/test/browser/browser_UsageTelemetry_urlbar_remotetab.js
toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js
--- a/accessible/tests/browser/general/browser_test_urlbar.js
+++ b/accessible/tests/browser/general/browser_test_urlbar.js
@@ -8,17 +8,21 @@ const {UrlbarTestUtils} = ChromeUtils.im
 
 // Checking that the awesomebar popup gets COMBOBOX_LIST role instead of
 // LISTBOX, since its parent is a <panel> (see Bug 1422465)
 add_task(async function testAutocompleteRichResult() {
   let tab = await openNewTab("data:text/html;charset=utf-8,");
   let accService = await initAccessibilityService();
 
   info("Opening the URL bar and entering a key to show the PopupAutoCompleteRichResult panel");
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "a", waitForFocus);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "a"
+  });
 
   info("Waiting for accessibility to be created for the richlistbox");
   let resultsView;
   if (UrlbarPrefs.get("quantumbar")) {
     resultsView = gURLBar.view.panel.querySelector("#urlbarView-results");
     await BrowserTestUtils.waitForCondition(() => accService.getAccessibleFor(resultsView));
   } else {
     let urlbarPopup = document.getElementById("PopupAutoCompleteRichResult");
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -729,18 +729,21 @@ async function runUrlbarTest(useAwesomeb
       let searchTerm = "ows-10";
       for (let i = 0; i < searchTerm.length; ++i) {
         let char = searchTerm[i];
         EventUtils.synthesizeKey(char, {}, win);
         await UrlbarTestUtils.promiseSearchComplete(win);
         await waitExtra();
       }
     } else {
-      await UrlbarTestUtils.promiseAutocompleteResultPopup(win, URLBar.value,
-                                                           SimpleTest.waitForFocus);
+      await UrlbarTestUtils.promiseAutocompleteResultPopup({
+        window: win,
+        waitForFocus: SimpleTest.waitForFocus,
+        value: URLBar.value,
+      });
       await waitExtra();
     }
 
     await UrlbarTestUtils.promisePopupClose(win);
   };
 
   let dropmarkerRect = win.document.getAnonymousElementByAttribute(URLBar.textbox,
     "anonid", "historydropmarker").getBoundingClientRect();
--- a/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js
+++ b/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js
@@ -5,18 +5,22 @@
 ChromeUtils.defineModuleGetter(this, "PlacesUtils",
                                "resource://gre/modules/PlacesUtils.jsm");
 ChromeUtils.defineModuleGetter(this, "UrlbarTestUtils",
                                "resource://testing-common/UrlbarTestUtils.jsm");
 
 const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
-function promiseAutocompleteResultPopup(inputText) {
-  return UrlbarTestUtils.promiseAutocompleteResultPopup(window, inputText, waitForFocus);
+function promiseAutocompleteResultPopup(value) {
+  return UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value,
+  });
 }
 
 async function addBookmark(bookmark) {
   if (bookmark.keyword) {
     await PlacesUtils.keywords.insert({
       keyword: bookmark.keyword,
       url: bookmark.url,
     });
@@ -126,17 +130,17 @@ add_task(async function test_webnavigati
       permissions: ["webNavigation"],
     },
   });
 
   await extension.startup();
   await SimpleTest.promiseFocus(window);
 
   await extension.awaitMessage("ready");
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "http://example.com/?q=typedClosed", waitForFocus);
+  await promiseAutocompleteResultPopup("http://example.com/?q=typedClosed");
   await UrlbarTestUtils.promiseSearchComplete(window);
   // Closing the popup forces a different code route that handles no results
   // being displayed.
   await UrlbarTestUtils.promisePopupClose(window);
   EventUtils.synthesizeKey("VK_RETURN", {});
 
   await extension.awaitFinish("webNavigation.from_address_bar.typed");
 
--- a/browser/components/search/test/browser/browser_oneOffContextMenu_setDefault.js
+++ b/browser/components/search/test/browser/browser_oneOffContextMenu_setDefault.js
@@ -142,17 +142,21 @@ async function openPopupAndGetEngineButt
   // We have to open the popups in differnt ways.
   if (isSearch) {
     // Open the popup.
     let promise = promiseEvent(popup, "popupshown");
     // Use the search icon to avoid hitting the network.
     EventUtils.synthesizeMouseAtCenter(searchIcon, {});
     await promise;
   } else {
-    await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "a", waitForFocus);
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      waitForFocus,
+      value: "a",
+    });
   }
 
   const contextMenu = oneOffInstance.contextMenuPopup;
   const oneOffButtons = oneOffInstance.buttons;
 
   // Get the one-off button for the test engine.
   let oneOffButton;
   for (let node of oneOffButtons.children) {
--- a/browser/components/sessionstore/test/browser_tabs_in_urlbar.js
+++ b/browser/components/sessionstore/test/browser_tabs_in_urlbar.js
@@ -64,17 +64,21 @@ add_task(async function test_unrestored_
     // after that has fired for all tabs. Since 1 tab will be restored though, we
     // also need to wait for 1 SSTabRestored since currentURI will be set, unset, then set.
     gBrowser.tabContainer.addEventListener("SSTabRestoring", handleEvent, true);
     gBrowser.tabContainer.addEventListener("SSTabRestored", handleEvent, true);
     ss.setBrowserState(JSON.stringify(state));
   });
 
   info("Searching open pages.");
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, RESTRICT_TOKEN_OPENPAGE, waitForFocus);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: RESTRICT_TOKEN_OPENPAGE,
+  });
   const total = UrlbarTestUtils.getResultCount(window);
   info(`Found ${total} matches`);
   const quantumbar = UrlbarPrefs.get("quantumbar");
 
   // Check to see the expected uris and titles match up (in any order)
   for (let i = 0; i < total; i++) {
     const result = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
     if (result.heuristic) {
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -141,19 +141,19 @@ class UrlbarController {
   receiveResults(queryContext) {
     if (queryContext.lastResultCount < 1 && queryContext.results.length >= 1) {
       TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT, queryContext);
     }
     if (queryContext.lastResultCount < 6 && queryContext.results.length >= 6) {
       TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS, queryContext);
     }
 
-    if (queryContext.lastResultCount == 0) {
-      if (queryContext.results.length && queryContext.results[0].autofill) {
-        this.input.setValueFromResult(queryContext.results[0]);
+    if (queryContext.lastResultCount == 0 && queryContext.results.length) {
+      if (queryContext.results[0].autofill) {
+        this.input.autofillFirstResult(queryContext.results[0]);
       }
       // The first time we receive results try to connect to the heuristic
       // result.
       this.speculativeConnect(queryContext, 0, "resultsadded");
     }
 
     this._notify("onQueryResults", queryContext);
     // Update lastResultCount after notifying, so the view can use it.
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -92,16 +92,19 @@ class UrlbarInput {
     this.lastQueryContextPromise = Promise.resolve();
     this._actionOverrideKeyCount = 0;
     this._autofillPlaceholder = "";
     this._lastSearchString = "";
     this._resultForCurrentValue = null;
     this._suppressStartQuery = false;
     this._untrimmedValue = "";
 
+    // This exists only for tests.
+    this._enableAutofillPlaceholder = true;
+
     // Forward textbox methods and properties.
     const METHODS = ["addEventListener", "removeEventListener",
       "setAttribute", "hasAttribute", "removeAttribute", "getAttribute",
       "select"];
     const READ_ONLY_PROPERTIES = ["inputField", "editor"];
     const READ_WRITE_PROPERTIES = ["placeholder", "readOnly",
       "selectionStart", "selectionEnd"];
 
@@ -500,32 +503,65 @@ class UrlbarInput {
         this.value = this._getValueFromResult(result);
       }
     }
     this._resultForCurrentValue = result;
 
     // Also update userTypedValue. See bug 287996.
     this.window.gBrowser.userTypedValue = this.value;
 
-    // The value setter clobbers the actiontype attribute, so update this after that.
+    // The value setter clobbers the actiontype attribute, so update this after
+    // that.
     if (result) {
       switch (result.type) {
         case UrlbarUtils.RESULT_TYPE.TAB_SWITCH:
           this.setAttribute("actiontype", "switchtab");
           break;
         case UrlbarUtils.RESULT_TYPE.OMNIBOX:
           this.setAttribute("actiontype", "extension");
           break;
       }
     }
 
     return !!canonizedUrl;
   }
 
   /**
+   * Called by the controller when the first result of a new search is received.
+   * If it's an autofill result, then it may need to be autofilled, subject to a
+   * few restrictions.
+   *
+   * @param {UrlbarResult} result
+   *   The first result.
+   */
+  autofillFirstResult(result) {
+    if (!result.autofill) {
+      return;
+    }
+
+    let isPlaceholderSelected =
+      this.selectionEnd == this._autofillPlaceholder.length &&
+      this.selectionStart == this._lastSearchString.length &&
+      this._autofillPlaceholder.toLocaleLowerCase()
+        .startsWith(this._lastSearchString.toLocaleLowerCase());
+
+    // Don't autofill if there's already a selection (with one caveat described
+    // next) or the cursor isn't at the end of the input.  But if there is a
+    // selection and it's the autofill placeholder value, then do autofill.
+    if (!isPlaceholderSelected &&
+        (this.selectionStart != this.selectionEnd ||
+         this.selectionEnd != this._lastSearchString.length)) {
+      return;
+    }
+
+    let { value, selectionStart, selectionEnd } = result.autofill;
+    this._autofillValue(value, selectionStart, selectionEnd);
+  }
+
+  /**
    * 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.
@@ -742,17 +778,18 @@ class UrlbarInput {
     // it starts with the new search string, and we shouldn't autofill anything
     // if the caret isn't at the end of the input.
     if (!allowAutofill ||
         this._autofillPlaceholder.length <= value.length ||
         !this._autofillPlaceholder.toLocaleLowerCase()
           .startsWith(value.toLocaleLowerCase())) {
       this._autofillPlaceholder = "";
     } else if (this._autofillPlaceholder &&
-               this.selectionEnd == this.value.length) {
+               this.selectionEnd == this.value.length &&
+               this._enableAutofillPlaceholder) {
       let autofillValue =
         value + this._autofillPlaceholder.substring(value.length);
       this._autofillValue(autofillValue, value.length, autofillValue.length);
     }
 
     return allowAutofill;
   }
 
@@ -1164,17 +1201,17 @@ class UrlbarInput {
 
   /**
    * This notifies observers that the user has entered or selected something in
    * the URL bar which will cause navigation.
    *
    * We use the observer service, so that we don't need to load extra facilities
    * if they aren't being used, e.g. WebNavigation.
    *
-   * @param {UrlbarResult} [result]
+   * @param {UrlbarResult} result
    *   The result that was selected, if any.
    */
   _notifyStartNavigation(result) {
     Services.obs.notifyObservers({result}, "urlbar-user-start-navigation");
   }
 
   // Event handlers below.
 
--- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm
+++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm
@@ -31,48 +31,62 @@ var UrlbarTestUtils = {
       if (typeof restoreAnimations == "function") {
         restoreAnimationsFn();
       }
     });
   },
 
   /**
    * Starts a search for a given string and waits for the search to be complete.
-   * @param {object} win The window containing the urlbar
-   * @param {string} inputText the search string
-   * @param {function} waitForFocus The Simpletest function
-   * @param {boolean} fireInputEvent whether an input event should be used when
-   *        starting the query (necessary to set userTypedValued)
+   * @param {object} options.window The window containing the urlbar
+   * @param {string} options.value the search string
+   * @param {function} options.waitForFocus The SimpleTest function
+   * @param {boolean} [options.fireInputEvent] whether an input event should be
+   *        used when starting the query (simulates the user's typing, sets
+   *        userTypedValued, etc.)
+   * @param {number} [options.selectionStart] The input's selectionStart
+   * @param {number} [options.selectionEnd] The input's selectionEnd
    */
-  async promiseAutocompleteResultPopup(win, inputText, waitForFocus, fireInputEvent = false) {
-    let urlbar = getUrlbarAbstraction(win);
+  async promiseAutocompleteResultPopup({
+    window,
+    value,
+    waitForFocus,
+    fireInputEvent = false,
+    selectionStart = -1,
+    selectionEnd = -1,
+  } = {}) {
+    let urlbar = getUrlbarAbstraction(window);
     let restoreAnimationsFn = urlbar.disableAnimations();
-    await new Promise(resolve => waitForFocus(resolve, win));
+    await new Promise(resolve => waitForFocus(resolve, window));
     let lastSearchString = urlbar.lastSearchString;
     urlbar.focus();
-    urlbar.value = inputText;
+    urlbar.value = value;
+    if (selectionStart >= 0 && selectionEnd >= 0) {
+      urlbar.selectionEnd = selectionEnd;
+      urlbar.selectionStart = selectionStart;
+    }
     if (fireInputEvent) {
       // This is necessary to get the urlbar to set gBrowser.userTypedValue.
       urlbar.fireInputEvent();
     } else {
-      win.gURLBar.setAttribute("pageproxystate", "invalid");
+      window.gURLBar.setAttribute("pageproxystate", "invalid");
     }
     // An input event will start a new search, with a couple of exceptions, so
     // be careful not to call startSearch if we fired an input event since that
     // would start two searches.  The first exception is when the new search and
     // old search are the same.  Many tests do consecutive searches with the
     // same string and expect new searches to start, so call startSearch
     // directly then.  The second exception is when searching with the legacy
     // urlbar and an empty string.
     if (!fireInputEvent ||
-        inputText == lastSearchString ||
-        (!urlbar.quantumbar && !inputText)) {
-      urlbar.startSearch(inputText);
+        value == lastSearchString ||
+        (!urlbar.quantumbar && !value)) {
+      urlbar.startSearch(value, selectionStart, selectionEnd);
     }
-    return this.promiseSearchComplete(win, restoreAnimationsFn);
+    return this.promiseSearchComplete(window, restoreAnimationsFn);
   },
 
   /**
    * Waits for a result to be added at a certain index. Since we implement lazy
    * results replacement, even if we have a result at an index, it may be
    * related to the previous query, this methods ensures the result is current.
    * @param {object} win The window containing the urlbar
    * @param {number} index The index to look for
@@ -334,19 +348,23 @@ class UrlbarAbstraction {
   get oneOffSearchButtonsVisible() {
     if (!this.quantumbar) {
       return this.window.getComputedStyle(this.oneOffSearchButtons.container).display != "none";
     }
 
     return this.oneOffSearchButtons.style.display != "none";
   }
 
-  startSearch(text) {
+  startSearch(text, selectionStart = -1, selectionEnd = -1) {
     if (this.quantumbar) {
       this.urlbar.value = text;
+      if (selectionStart >= 0 && selectionEnd >= 0) {
+        this.urlbar.selectionEnd = selectionEnd;
+        this.urlbar.selectionStart = selectionStart;
+      }
       this.urlbar.setAttribute("pageproxystate", "invalid");
       this.urlbar.startQuery();
     } else {
       // Force the controller to do consecutive searches for the same string by
       // calling resetInternalState.
       this.urlbar.controller.resetInternalState();
       this.urlbar.controller.startSearch(text);
     }
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -21,20 +21,22 @@ skip-if = true # Bug 1524539 - need to f
 [browser_autocomplete_enter_race.js]
 [browser_autocomplete_no_title.js]
 [browser_autocomplete_readline_navigation.js]
 skip-if = os != "mac" # Mac only feature
 [browser_autocomplete_tag_star_visibility.js]
 [browser_autoFill_backspaced.js]
 [browser_autoFill_canonize.js]
 [browser_autoFill_caretNotAtEnd.js]
+[browser_autoFill_firstResult.js]
 [browser_autoFill_placeholder.js]
 [browser_autoFill_preserve.js]
 [browser_autoFill_trimURLs.js]
 [browser_autoFill_typed.js]
+[browser_autoFill_undo.js]
 [browser_canonizeURL.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_caret_navigation.js]
 [browser_downArrowKeySearch.js]
 [browser_dragdropURL.js]
 [browser_dropmarker.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_autoFill_firstResult.js
@@ -0,0 +1,208 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This test makes sure that autofilling the first result of a new search works
+// correctly: autofill happens when it should and doesn't when it shouldn't.
+
+"use strict";
+
+add_task(async function init() {
+  await PlacesUtils.bookmarks.eraseEverything();
+  await PlacesUtils.history.clear();
+  await PlacesTestUtils.addVisits([
+    "http://example.com/",
+  ]);
+
+  // Disable placeholder completion.  The point of this test is to make sure the
+  // first result is autofilled (or not) correctly.  Autofilling the placeholder
+  // before the search starts interferes with that.
+  gURLBar._enableAutofillPlaceholder = false;
+  registerCleanupFunction(async () => {
+    gURLBar._enableAutofillPlaceholder = true;
+  });
+});
+
+// The first result should be autofilled when all conditions are met.  This also
+// does a sanity check to make sure that placeholder autofill is correctly
+// disabled, which is helpful for all tasks here and is why this one is first.
+add_task(async function successfulAutofill() {
+  // Do a simple search that should autofill.  This will also set up the
+  // autofill placeholder string, which next we make sure is *not* autofilled.
+  await doInitialAutofillSearch();
+
+  // As a sanity check, do another search to make sure the placeholder is *not*
+  // autofilled.  Make sure it's not autofilled by checking the input value and
+  // selection *before* the search completes.  If placeholder autofill was not
+  // correctly disabled, then these assertions will fail.
+
+  gURLBar.value = "exa";
+  UrlbarTestUtils.fireInputEvent(window);
+
+  // before the search completes: no autofill
+  Assert.equal(gURLBar.value, "exa");
+  Assert.equal(gURLBar.selectionStart, "exa".length);
+  Assert.equal(gURLBar.selectionEnd, "exa".length);
+
+  await UrlbarTestUtils.promiseSearchComplete(window);
+
+  // after the search completes: successful autofill
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "exa".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+});
+
+// The first result should not be autofilled when it's not an autofill result.
+add_task(async function firstResultNotAutofill() {
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "foo",
+    fireInputEvent: true,
+  });
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(!details.autofill);
+  Assert.equal(gURLBar.value, "foo");
+  Assert.equal(gURLBar.selectionStart, "foo".length);
+  Assert.equal(gURLBar.selectionEnd, "foo".length);
+});
+
+// The first result should *not* be autofilled when the placeholder is not
+// selected, the selection is empty, and the caret is *not* at the end of the
+// search string.
+add_task(async function caretNotAtEndOfSearchString() {
+  // To set up the placeholder, do an initial search that triggers autofill.
+  await doInitialAutofillSearch();
+
+  // Now do another search but set the caret to somewhere else besides the end
+  // of the new search string.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "exam",
+    selectionStart: "exa".length,
+    selectionEnd: "exa".length,
+  });
+
+  // The first result should be an autofill result, but it should not have been
+  // autofilled.
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "exam");
+  Assert.equal(gURLBar.selectionStart, "exa".length);
+  Assert.equal(gURLBar.selectionEnd, "exa".length);
+
+  await cleanUp();
+});
+
+// The first result should *not* be autofilled when the placeholder is not
+// selected, the selection is *not* empty, and the caret is at the end of the
+// search string.
+add_task(async function selectionNotEmpty() {
+  // To set up the placeholder, do an initial search that triggers autofill.
+  await doInitialAutofillSearch();
+
+  // Now do another search.  Set the selection end at the end of the search
+  // string, but make the selection non-empty.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "exam",
+    selectionStart: "exa".length,
+    selectionEnd: "exam".length,
+  });
+
+  // The first result should be an autofill result, but it should not have been
+  // autofilled.
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "exam");
+  Assert.equal(gURLBar.selectionStart, "exa".length);
+  Assert.equal(gURLBar.selectionEnd, "exam".length);
+
+  await cleanUp();
+});
+
+// The first result should be autofilled when the placeholder is not selected,
+// the selection is empty, and the caret is at the end of the search string.
+add_task(async function successfulAutofillAfterSettingPlaceholder() {
+  // To set up the placeholder, do an initial search that triggers autofill.
+  await doInitialAutofillSearch();
+
+  // Now do another search.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "exam",
+    selectionStart: "exam".length,
+    selectionEnd: "exam".length,
+  });
+
+  // It should be autofilled.
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "exam".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+
+  await cleanUp();
+});
+
+// The first result should be autofilled when the placeholder *is* selected --
+// more precisely, when the portion of the placeholder after the new search
+// string is selected.
+add_task(async function successfulAutofillPlaceholderSelected() {
+  // To set up the placeholder, do an initial search that triggers autofill.
+  await doInitialAutofillSearch();
+
+  // Now do another search and select the portion of the placeholder after the
+  // new search string.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "exam",
+    selectionStart: "exam".length,
+    selectionEnd: "example.com/".length,
+  });
+
+  // It should be autofilled.
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "exam".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+
+  await cleanUp();
+});
+
+async function doInitialAutofillSearch() {
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "ex",
+    fireInputEvent: true,
+  });
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "ex".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+}
+
+async function cleanUp() {
+  // In some cases above, a test task searches for "exam" at the end, and then
+  // the next task searches for "ex".  Autofill results will not be allowed in
+  // the next task in that case since the old search string starts with the new
+  // search string.  To prevent one task from interfering with the next, do a
+  // search that changes the search string.  Also close the popup while we're
+  // here, although that's not really necessary.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "reset last search string",
+  });
+  await UrlbarTestUtils.promisePopupClose(window, () => {
+    EventUtils.synthesizeKey("KEY_Escape");
+  });
+}
--- a/browser/components/urlbar/tests/browser/browser_autoFill_typed.js
+++ b/browser/components/urlbar/tests/browser/browser_autoFill_typed.js
@@ -150,20 +150,25 @@ async function typeAndCheck(values) {
          `substring="${expectedInputValue.substring(0, i + 1)}"`);
     EventUtils.synthesizeKey(char);
     if (i == 0 && char == "@") {
       // A single "@" doesn't trigger autofill, so skip the checks below.  (It
       // shows all the @ aliases.)
       continue;
     }
     await UrlbarTestUtils.promiseSearchComplete(window);
+    let restIsSpaces = !expectedInputValue.substring(i + 1).trim();
+    if (restIsSpaces && !UrlbarPrefs.get("quantumbar")) {
+      // See below.  In addition to that, in awesomebar, after typing the final
+      // character, autofill incorrectly doesn't include the trailing space.
+      expectedInputValue = expectedInputValue.trim();
+    }
     Assert.equal(gURLBar.value, expectedInputValue);
     Assert.equal(gURLBar.selectionStart, i + 1);
     Assert.equal(gURLBar.selectionEnd, expectedInputValue.length);
-    let restIsSpaces = !expectedInputValue.substring(i + 1).trim();
     if (restIsSpaces) {
       // Autofilled @ aliases have a trailing space.  We should check that the
       // space is autofilled when each preceding character is typed, but once
       // the final non-space char is typed, autofill actually stops and the
       // trailing space is not autofilled.  (Which is maybe not the way it
       // should work...)  Skip the check below.
       continue;
     }
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_autoFill_undo.js
@@ -0,0 +1,53 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This test checks the behavior of text undo (Ctrl-Z, cmd_undo) in regard to
+// autofill.
+
+"use strict";
+
+add_task(async function test() {
+  await PlacesUtils.bookmarks.eraseEverything();
+  await PlacesUtils.history.clear();
+  await PlacesTestUtils.addVisits([
+    "http://example.com/",
+  ]);
+
+  // Search for "ex".  It should autofill to example.com/.
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "ex",
+    fireInputEvent: true,
+  });
+  let details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "ex".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+
+  // Type an x.
+  EventUtils.synthesizeKey("x");
+
+  // Nothing should have been autofilled.
+  await UrlbarTestUtils.promiseSearchComplete(window);
+  details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(!details.autofill);
+  Assert.equal(gURLBar.value, "exx");
+  Assert.equal(gURLBar.selectionStart, "exx".length);
+  Assert.equal(gURLBar.selectionEnd, "exx".length);
+
+  // Undo the typed x.
+  goDoCommand("cmd_undo");
+
+  // The text should be restored to ex[ample.com/] (with the part in brackets
+  // autofilled and selected).
+  await UrlbarTestUtils.promiseSearchComplete(window);
+  details = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(details.autofill);
+  Assert.equal(gURLBar.value, "example.com/");
+  Assert.equal(gURLBar.selectionStart, "ex".length);
+  Assert.equal(gURLBar.selectionEnd, "example.com/".length);
+
+  await PlacesUtils.history.clear();
+});
--- a/browser/components/urlbar/tests/browser/browser_autocomplete_autoselect.js
+++ b/browser/components/urlbar/tests/browser/browser_autocomplete_autoselect.js
@@ -54,17 +54,17 @@ add_task(async function() {
   let visits = [];
   repeat(maxResults, i => {
     visits.push({
       uri: makeURI("http://example.com/autocomplete/?" + i),
     });
   });
   await PlacesTestUtils.addVisits(visits);
 
-  await promiseAutocompleteResultPopup("example.com/autocomplete");
+  await promiseAutocompleteResultPopup("example.com/autocomplete", window, true);
 
   let resultCount = await UrlbarTestUtils.getResultCount(window);
 
   Assert.equal(resultCount, maxResults,
     "Should get the expected amount of results");
   assertSelected(0);
 
   info("Key Down to select the next item");
--- a/browser/components/urlbar/tests/browser/browser_switchToTab_closes_newtab.js
+++ b/browser/components/urlbar/tests/browser/browser_switchToTab_closes_newtab.js
@@ -26,17 +26,21 @@ add_task(async function test_switchToTab
     "TabClose", false, event => {
       return event.originalTarget == testTab;
     });
   let tabSelectPromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer,
     "TabSelect", false, event => {
       return event.originalTarget == baseTab;
     });
 
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "dummy", waitForFocus);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "dummy",
+  });
 
   // The first result is the heuristic, the second will be the switch to tab.
   let element = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 1);
   EventUtils.synthesizeMouseAtCenter(element, {}, window);
 
   await Promise.all([tabSelectPromise, tabClosePromise]);
 
   // Confirm that the selected tab is now the base tab
--- a/browser/components/urlbar/tests/browser/browser_switchToTab_fullUrl_repeatedKeydown.js
+++ b/browser/components/urlbar/tests/browser/browser_switchToTab_fullUrl_repeatedKeydown.js
@@ -17,17 +17,22 @@ add_task(async function test_switchToTab
   let testTab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
 
   // Functions for TabClose and TabSelect
   let tabClosePromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer,
     "TabClose", false, event => event.target == testTab);
   let tabSelectPromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer,
     "TabSelect", false, event => event.target == baseTab);
 
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, TEST_URL, waitForFocus, true);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: TEST_URL,
+    fireInputEvent: true,
+  });
   // The first result is the heuristic, the second will be the switch to tab.
   await UrlbarTestUtils.waitForAutocompleteResultAt(window, 1);
 
   // Simulate a long press, on some platforms (Windows) it can generate multiple
   // keydown events.
   EventUtils.synthesizeKey("VK_SHIFT", { type: "keydown", repeat: 3 });
   EventUtils.synthesizeKey("VK_SHIFT", { type: "keyup" });
 
--- a/browser/components/urlbar/tests/browser/head-common.js
+++ b/browser/components/urlbar/tests/browser/head-common.js
@@ -50,21 +50,25 @@ async function withHttpServer(details = 
     server = null;
   }
 }
 
 function promiseSearchComplete(win = window, dontAnimate = false) {
   return UrlbarTestUtils.promiseSearchComplete(win, dontAnimate);
 }
 
-function promiseAutocompleteResultPopup(inputText,
+function promiseAutocompleteResultPopup(value,
                                         win = window,
                                         fireInputEvent = false) {
-  return UrlbarTestUtils.promiseAutocompleteResultPopup(win, inputText,
-    waitForFocus, fireInputEvent);
+  return UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window: win,
+    waitForFocus,
+    value,
+    fireInputEvent,
+  });
 }
 
 async function waitForAutocompleteResultAt(index) {
   return UrlbarTestUtils.waitForAutocompleteResultAt(window, index);
 }
 
 function promiseSuggestionsPresent(msg = "") {
   return UrlbarTestUtils.promiseSuggestionsPresent(window, msg);
--- a/browser/components/urlbar/tests/legacy/browser.ini
+++ b/browser/components/urlbar/tests/legacy/browser.ini
@@ -46,16 +46,18 @@ skip-if = (verify && !debug && (os == 'w
 [../browser/browser_autocomplete_enter_race.js]
 [../browser/browser_autocomplete_no_title.js]
 [../browser/browser_autocomplete_readline_navigation.js]
 skip-if = os != "mac" # Mac only feature
 [../browser/browser_autoFill_backspaced.js]
 [../browser/browser_autoFill_canonize.js]
 [../browser/browser_autoFill_preserve.js]
 [../browser/browser_autoFill_trimURLs.js]
+[../browser/browser_autoFill_typed.js]
+[../browser/browser_autoFill_undo.js]
 [../browser/browser_autocomplete_tag_star_visibility.js]
 [../browser/browser_canonizeURL.js]
 [../browser/browser_dragdropURL.js]
 [../browser/browser_ime_composition.js]
 [../browser/browser_keywordBookmarklets.js]
 [../browser/browser_keepStateAcrossTabSwitches.js]
 [../browser/browser_keyword_override.js]
 [../browser/browser_keywordSearch.js]
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
@@ -18,18 +18,23 @@ const ONEOFF_URLBAR_PREF = "browser.urlb
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   SearchTelemetry: "resource:///modules/SearchTelemetry.jsm",
   UrlbarTestUtils: "resource://testing-common/UrlbarTestUtils.jsm",
   URLBAR_SELECTED_RESULT_TYPES: "resource:///modules/BrowserUsageTelemetry.jsm",
   URLBAR_SELECTED_RESULT_METHODS: "resource:///modules/BrowserUsageTelemetry.jsm",
 });
 
-function searchInAwesomebar(inputText, win = window) {
-  return UrlbarTestUtils.promiseAutocompleteResultPopup(win, inputText, waitForFocus, true);
+function searchInAwesomebar(value, win = window) {
+  return UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window: win,
+    waitForFocus,
+    value,
+    fireInputEvent: true,
+  });
 }
 
 /**
  * Click one of the entries in the urlbar suggestion popup.
  *
  * @param {String} resultTitle
  *        The title of the result to click on.
  * @param {Number} button [optional]
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar_extension.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar_extension.js
@@ -115,18 +115,22 @@ add_task(async function test_extension()
   });
 
   await extension.startup();
 
   const histograms = snapshotHistograms();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank");
 
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "omniboxtest ",
-    waitForFocus, true);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "omniboxtest ",
+    fireInputEvent: true,
+  });
   EventUtils.synthesizeKey("KEY_Enter");
 
   assertSearchTelemetryEmpty(histograms.search_hist);
   assertHistogramResults(histograms, "extension", 0,
     URLBAR_SELECTED_RESULT_METHODS.enter);
 
   await extension.unload();
   BrowserTestUtils.removeTab(tab);
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar_places.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar_places.js
@@ -15,18 +15,23 @@ const TEST_URL = getRootDirectory(gTestP
   .replace("chrome://mochitests/content", "http://mochi.test:8888");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   UrlbarTestUtils: "resource://testing-common/UrlbarTestUtils.jsm",
   URLBAR_SELECTED_RESULT_TYPES: "resource:///modules/BrowserUsageTelemetry.jsm",
   URLBAR_SELECTED_RESULT_METHODS: "resource:///modules/BrowserUsageTelemetry.jsm",
 });
 
-function searchInAwesomebar(inputText, win = window) {
-  return UrlbarTestUtils.promiseAutocompleteResultPopup(win, inputText, waitForFocus, true);
+function searchInAwesomebar(value, win = window) {
+  return UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window: win,
+    waitForFocus,
+    value,
+    fireInputEvent: true,
+  });
 }
 
 function assertSearchTelemetryEmpty(search_hist) {
   const scalars = TelemetryTestUtils.getProcessScalars("parent", true, false);
   Assert.ok(!(SCALAR_URLBAR in scalars), `Should not have recorded ${SCALAR_URLBAR}`);
 
   // Make sure SEARCH_COUNTS contains identical values.
   TelemetryTestUtils.assertKeyedHistogramSum(search_hist, "other-MozSearch.urlbar", undefined);
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar_remotetab.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar_remotetab.js
@@ -139,17 +139,22 @@ add_task(async function setup() {
 });
 
 add_task(async function test_remotetab() {
   const histograms = snapshotHistograms();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank");
 
   let p = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window, "example", waitForFocus, true);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "example",
+    fireInputEvent: true,
+  });
   EventUtils.synthesizeKey("KEY_ArrowDown");
   EventUtils.synthesizeKey("KEY_Enter");
   await p;
 
   assertSearchTelemetryEmpty(histograms.search_hist);
   assertHistogramResults(histograms, "remotetab", 1,
     URLBAR_SELECTED_RESULT_METHODS.arrowEnterSelection);
 
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js
@@ -80,19 +80,21 @@ add_task(async function test_popup_url()
 
   let visits = [];
 
   for (let i = 0; i < maxResults; i++) {
     visits.push({uri: makeURI("http://example.com/autocomplete/?" + i)});
   }
 
   await PlacesTestUtils.addVisits(visits);
-  await UrlbarTestUtils.promiseAutocompleteResultPopup(window,
-                                                       "example.com/autocomplete",
-                                                       waitForFocus);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "example.com/autocomplete",
+  });
   await UrlbarTestUtils.waitForAutocompleteResultAt(window, maxResults - 1);
 
   Assert.equal(UrlbarTestUtils.getResultCount(window), maxResults,
                "Should get maxResults=" + maxResults + " results");
 
   let popup = UrlbarTestUtils.getPanel(window);
   let popupCS = window.getComputedStyle(popup);