Bug 1593964 - part 1 - Stop storing autofilled text in userTypedValue. r=dao

The original fix for Bug 287996 intended to store the url as userTypedValue when
it was confirmed in the UI and about to start loading, but it threw the baby out
with the bathwater. By storing any selection-completed and autofilled value as
userTypedValue, it made impossible to properly return to the typed text.
Currently, if autofill happens, switching back to the tab where it happened
shows the autofilled value instead of what the user typed.
This also makes impossible to reopen the view, because the search string changed.
Because now we properly set userTypedValue in _loadURL, we can remove this old
workaround.
Note though that this breaks selecting a url (without confirmation), switching tab,
then going back to the original tab and expecting the url to be set.
While it's possible to support that case, we think it's more important to go
back to what was typed, and eventually reopen the view. It's also less confusing
for the user, because the shown url has no relation with the currently loaded page.

For similar reasons, blurring the urlbar should restore the user typed value
if we autofilled a value, to avoid overriding the search string with our own.
And because of this, we should not clear the view every time set a value, or we
could clear valid cached results. It's better to do it on an actual input event.

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

--HG--
extra : moz-landing-system : lando

browser/components/urlbar/UrlbarInput.jsm

@@ -718,9 +718,6 @@ class UrlbarInput {
     }
     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.
     if (result) {
@@ -1089,9 +1086,6 @@ class UrlbarInput {
     this.inputField.value = val;
     this.formatValue();
     this.removeAttribute("actiontype");
-    if (!this.view.isOpen) {
-      this.view.clear();
-    }
 
     // Dispatch ValueChange event for accessibility.
     let event = this.document.createEvent("Events");
@@ -1674,14 +1668,14 @@ class UrlbarInput {
     this.removeAttribute("focused");
     this.endLayoutExtend();
 
+    if (this._autofillPlaceholder && this.window.gBrowser.userTypedValue) {
+      // Restore value to the last typed one, removing any autofilled portion.
+      this.value = this.window.gBrowser.userTypedValue;
+    }
+
     this.formatValue();
     this._resetSearchState();
 
-    // Clear selection unless we are switching application windows.
-    if (this.document.activeElement != this.inputField) {
-      this.selectionStart = this.selectionEnd = 0;
-    }
-
     // In certain cases, like holding an override key and confirming an entry,
     // we don't key a keyup event for the override key, thus we make this
     // additional cleanup on blur.
@@ -1789,6 +1783,12 @@ class UrlbarInput {
           break;
         }
 
+        // Clear any previous selection unless we are focused, to ensure it
+        // doesn't affect drag selection.
+        if (this._focusedViaMousedown) {
+          this.selectionStart = this.selectionEnd = 0;
+        }
+
         if (event.detail == 2 && UrlbarPrefs.get("doubleClickSelectsAll")) {
           this.editor.selectAll();
           event.preventDefault();
@@ -1861,9 +1861,11 @@ class UrlbarInput {
     }
     this.removeAttribute("actiontype");
 
-    if (!value && this.view.isOpen) {
-      this.view.close();
+    if (!this.view.isOpen || !value) {
       this.view.clear();
+    }
+    if (this.view.isOpen && !value) {
+      this.view.close();
       return;
     }
 

browser/components/urlbar/tests/browser/browser.ini

@@ -147,6 +147,7 @@ support-files =
   searchSuggestionEngine.sjs
 [browser_urlbar_speculative_connect_not_with_client_cert.js]
 [browser_urlbar_suggestedIndex.js]
+[browser_urlbar_typed_value.js]
 [browser_urlbar_whereToOpen.js]
 [browser_urlbarAboutHomeLoading.js]
 [browser_urlbarCopying.js]

browser/components/urlbar/tests/browser/browser_autoFill_backspaced.js

@@ -11,7 +11,12 @@ async function test_autocomplete(data) {
   let { desc, typed, autofilled, modified, keys, type, onAutoFill } = data;
   info(desc);
 
-  await promiseAutocompleteResultPopup(typed);
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: typed,
+    fireInputEvent: true,
+  });
   Assert.equal(gURLBar.value, autofilled, "autofilled value is as expected");
   if (onAutoFill) {
     onAutoFill();
@@ -22,8 +27,7 @@ async function test_autocomplete(data) {
     let args = Array.isArray(key) ? key : [key];
     EventUtils.synthesizeKey(...args);
   }
-
-  await promiseSearchComplete();
+  await UrlbarTestUtils.promiseSearchComplete(window);
 
   Assert.equal(gURLBar.value, modified, "backspaced value is as expected");
 
@@ -132,8 +136,16 @@ add_task(async function() {
     onAutoFill: () => {
       gURLBar.blur();
       gURLBar.focus();
-      gURLBar.selectionStart = 1;
-      gURLBar.selectionEnd = 12;
+      Assert.equal(
+        gURLBar.selectionStart,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
+      Assert.equal(
+        gURLBar.selectionEnd,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
     },
   });
   await test_autocomplete({
@@ -141,18 +153,26 @@ add_task(async function() {
     typed: "ex",
     autofilled: "example.com/",
     modified: "e",
-    keys: ["KEY_Delete"],
+    keys: ["KEY_ArrowLeft", "KEY_Delete"],
     type: UrlbarUtils.RESULT_TYPE.SEARCH,
     onAutoFill: () => {
       gURLBar.blur();
       gURLBar.focus();
-      gURLBar.selectionStart = 1;
-      gURLBar.selectionEnd = 12;
+      Assert.equal(
+        gURLBar.selectionStart,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
+      Assert.equal(
+        gURLBar.selectionEnd,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
     },
   });
   await test_autocomplete({
     desc: "double BACK_SPACE after blur should search",
-    typed: "ex",
+    typed: "exa",
     autofilled: "example.com/",
     modified: "e",
     keys: ["KEY_Backspace", "KEY_Backspace"],
@@ -160,8 +180,16 @@ add_task(async function() {
     onAutoFill: () => {
       gURLBar.blur();
       gURLBar.focus();
-      gURLBar.selectionStart = 2;
-      gURLBar.selectionEnd = 12;
+      Assert.equal(
+        gURLBar.selectionStart,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
+      Assert.equal(
+        gURLBar.selectionEnd,
+        gURLBar.value.length,
+        "blur/focus should not change selection"
+      );
     },
   });
 

browser/components/urlbar/tests/browser/browser_urlbar_typed_value.js

@@ -0,0 +1,69 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This test ensures that the urlbar is restored to the typed value on blur.
+
+"use strict";
+
+add_task(async function setup() {
+  registerCleanupFunction(async function() {
+    Services.prefs.clearUserPref("browser.urlbar.autoFill");
+    gURLBar.handleRevert();
+    await PlacesUtils.history.clear();
+  });
+  Services.prefs.setBoolPref("browser.urlbar.autoFill", true);
+
+  await PlacesTestUtils.addVisits([
+    "http://example.com/",
+    "http://example.com/foo",
+  ]);
+});
+
+add_task(async function test_autofill() {
+  let typed = "ex";
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: typed,
+    fireInputEvent: true,
+  });
+  Assert.equal(gURLBar.value, "example.com/", "autofilled value as expected");
+  Assert.equal(gURLBar.selectionStart, typed.length);
+  Assert.equal(gURLBar.selectionEnd, gURLBar.value.length);
+
+  gURLBar.blur();
+  Assert.equal(gURLBar.value, typed, "Value should have been restored");
+  gURLBar.focus();
+  Assert.equal(gURLBar.value, typed, "Value should not have changed");
+  Assert.equal(gURLBar.selectionStart, typed.length);
+  Assert.equal(gURLBar.selectionEnd, typed.length);
+});
+
+add_task(async function test_complete_selection() {
+  let typed = "ex";
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: typed,
+    fireInputEvent: true,
+  });
+  Assert.equal(gURLBar.value, "example.com/", "autofilled value as expected");
+  Assert.equal(
+    UrlbarTestUtils.getResultCount(window),
+    2,
+    "Should have the correct number of matches"
+  );
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  Assert.equal(
+    gURLBar.value,
+    "example.com/foo",
+    "Value should have been completed"
+  );
+
+  gURLBar.blur();
+  Assert.equal(gURLBar.value, typed, "Value should have been restored");
+  gURLBar.focus();
+  Assert.equal(gURLBar.value, typed, "Value should not have changed");
+  Assert.equal(gURLBar.selectionStart, typed.length);
+  Assert.equal(gURLBar.selectionEnd, typed.length);
+});