Bug 1522278 - Use nsIEditActionListener to detect when the user deletes autofilled substrings. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Wed, 17 Apr 2019 15:19:23 +0000
changeset 469908 47050699cd5c391afa95189016a00fbb726dc6b1
parent 469907 65f9115900b0a9684655699858f5c9629d802a50
child 469909 4cda78f998e8c858fa9307800395d4a9214a66ef
push id83419
push userdwillcoxon@mozilla.com
push dateWed, 17 Apr 2019 18:11:30 +0000
treeherderautoland@4cda78f998e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1522278
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 1522278 - Use nsIEditActionListener to detect when the user deletes autofilled substrings. r=mak Differential Revision: https://phabricator.services.mozilla.com/D27637
browser/components/urlbar/UrlbarInput.jsm
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -87,16 +87,17 @@ class UrlbarInput {
     this.controller.setInput(this);
     this.view = new UrlbarView(this);
     this.valueIsTyped = false;
     this.userInitiatedFocus = false;
     this.isPrivate = PrivateBrowsingUtils.isWindowPrivate(this.window);
     this.lastQueryContextPromise = Promise.resolve();
     this._actionOverrideKeyCount = 0;
     this._autofillPlaceholder = "";
+    this._deletedEndOfAutofillPlaceholder = false;
     this._lastSearchString = "";
     this._resultForCurrentValue = null;
     this._suppressStartQuery = false;
     this._untrimmedValue = "";
 
     // This exists only for tests.
     this._enableAutofillPlaceholder = true;
 
@@ -177,16 +178,18 @@ class UrlbarInput {
    * Uninitializes this input object, detaching it from the inputField.
    */
   uninit() {
     for (let name of this._inputFieldEvents) {
       this.inputField.removeEventListener(name, this);
     }
     this.removeEventListener("mousedown", this);
 
+    this.editor.removeEditActionListener(this);
+
     this.view.panel.remove();
 
     this.inputField.controllers.removeControllerAt(0);
 
     if (Object.getOwnPropertyDescriptor(this, "valueFormatter").get) {
       this.valueFormatter.uninit();
     }
 
@@ -663,16 +666,41 @@ class UrlbarInput {
   /**
    * Remove the hidden focus styles.
    * This is used by Activity Stream and about:privatebrowsing for search hand-off.
    */
   removeHiddenFocus() {
     this.textbox.classList.remove("hidden-focus");
   }
 
+  /**
+   * nsIEditActionListener method implementation.  We use this to detect when
+   * the user deletes autofilled substrings.
+   *
+   * There is also a DidDeleteSelection method, but it's called before the input
+   * event is fired.  So the order is: WillDeleteSelection, DidDeleteSelection,
+   * input event.  Further, in DidDeleteSelection, the passed-in selection
+   * object is the same as the object passed to WillDeleteSelection, but by that
+   * point its properties have been adjusted to account for the deletion.  For
+   * example, the endOffset property of its range will be smaller than it was in
+   * WillDeleteSelection.  Therefore we compute whether the user deleted the
+   * autofilled substring here in WillDeleteSelection instead of deferring it to
+   * when we handle the input event.
+   *
+   * @param {Selection} selection
+   *   The Selection object.
+   */
+  WillDeleteSelection(selection) {
+    this._deletedEndOfAutofillPlaceholder =
+      selection &&
+      selection.getRangeAt(0).endOffset ==
+        this._autofillPlaceholder.length &&
+      this._autofillPlaceholder.endsWith(String(selection));
+  }
+
   // Getters and Setters below.
 
   get focused() {
     return this.textbox.getAttribute("focused") == "true";
   }
 
   get goButton() {
     return this.document.getAnonymousElementByAttribute(this.textbox, "anonid",
@@ -1268,16 +1296,19 @@ class UrlbarInput {
   }
 
   _on_input() {
     let value = this.textValue;
     this.valueIsTyped = true;
     this._untrimmedValue = value;
     this.window.gBrowser.userTypedValue = value;
 
+    let deletedEndOfAutofillPlaceholder = this._deletedEndOfAutofillPlaceholder;
+    this._deletedEndOfAutofillPlaceholder = false;
+
     let compositionState = this._compositionState;
     let compositionClosedPopup = this._compositionClosedPopup;
 
     // Clear composition values if we're no more composing.
     if (this._compositionState != UrlbarUtils.COMPOSITION.COMPOSING) {
       this._compositionState = UrlbarUtils.COMPOSITION.NONE;
       this._compositionClosedPopup = false;
     }
@@ -1306,23 +1337,18 @@ class UrlbarInput {
     // and we didn't close the popup on composition start.
     if (compositionState == UrlbarUtils.COMPOSITION.COMPOSING ||
         (compositionState == UrlbarUtils.COMPOSITION.CANCELED &&
          !compositionClosedPopup)) {
       return;
     }
 
     let sameSearchStrings = value == this._lastSearchString;
-
-    // TODO (bug 1524550): Properly detect autofill removal, rather than
-    // guessing based on string prefixes.
     let deletedAutofilledSubstring =
-      sameSearchStrings &&
-      value.length < this._autofillPlaceholder.length &&
-      this._autofillPlaceholder.startsWith(value);
+      deletedEndOfAutofillPlaceholder && sameSearchStrings;
 
     // Don't search again when the new search would produce the same results.
     // If we're handling a composition input, we must continue the search
     // because we canceled the previous search on composition start.
     if (sameSearchStrings &&
         !deletedAutofilledSubstring &&
         compositionState == UrlbarUtils.COMPOSITION.NONE &&
         value.length > 0) {
@@ -1337,21 +1363,28 @@ class UrlbarInput {
       searchString: value,
       allowAutofill,
       lastKey: null,
       resetSearchState: false,
     });
   }
 
   _on_select(event) {
-    if (!Services.clipboard.supportsSelectionClipboard()) {
+    if (!this.window.windowUtils.isHandlingUserInput) {
+      // Register the editor listener we use to detect when the user deletes
+      // autofilled substrings.  The editor is destroyed and removes all its
+      // listeners at various surprising times, and autofill causes a non-user
+      // select, which is why we do this here instead of, for example, in the
+      // constructor.  addEditActionListener is idempotent, so it's OK to call
+      // it even when we're already registered.
+      this.editor.addEditActionListener(this);
       return;
     }
 
-    if (!this.window.windowUtils.isHandlingUserInput) {
+    if (!Services.clipboard.supportsSelectionClipboard()) {
       return;
     }
 
     let val = this._getSelectedValueForClipboard();
     if (!val) {
       return;
     }