Bug 1172937 - Action row doesn't always update correctly with unified autocomplete. r=adw

Original patch by Felipe Gomes <felipc@gmail.com>

--HG--
extra : commitid : 9zGMBGXYqlm

browser/base/content/test/general/browser.ini

@@ -452,6 +452,7 @@ skip-if = os == "linux" || e10s # Bug 1073339 - Investigate autocomplete test un
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarStop.js]
 [browser_urlbarTrimURLs.js]
+[browser_urlbar_autoFill_backspaced.js]
 [browser_urlbar_search_healthreport.js]
 [browser_urlbar_searchsettings.js]
 [browser_utilityOverlay.js]

browser/base/content/test/general/browser_urlbar_autoFill_backspaced.js

@@ -0,0 +1,148 @@
+/* This test ensures that backspacing autoFilled values still allows to
+ * confirm the remaining value.
+ */
+
+function* test_autocomplete(data) {
+  let {desc, typed, autofilled, modified, keys, action, onAutoFill} = data;
+  info(desc);
+
+  yield promiseAutocompleteResultPopup(typed);
+  is(gURLBar.value, autofilled, "autofilled value is as expected");
+  if (onAutoFill)
+    onAutoFill()
+
+  keys.forEach(key => EventUtils.synthesizeKey(key, {}));
+
+  is(gURLBar.value, modified, "backspaced value is as expected");
+
+  yield promiseSearchComplete();
+
+  ok(gURLBar.popup.richlistbox.children.length > 0, "Should get at least 1 result");
+  let result = gURLBar.popup.richlistbox.children[0];
+  let type = result.getAttribute("type");
+  let types = type.split(/\s+/);
+  ok(types.includes(action), `The type attribute "${type}" includes the expected action "${action}"`);
+
+  gURLBar.popup.hidePopup();
+  yield promisePopupHidden(gURLBar.popup);
+  gURLBar.blur();
+};
+
+add_task(function* () {
+  registerCleanupFunction(function* () {
+    Services.prefs.clearUserPref("browser.urlbar.unifiedcomplete");
+    Services.prefs.clearUserPref("browser.urlbar.autoFill");
+    gURLBar.handleRevert();
+    yield PlacesTestUtils.clearHistory();
+  });
+  Services.prefs.setBoolPref("browser.urlbar.unifiedcomplete", true);
+  Services.prefs.setBoolPref("browser.urlbar.autoFill", true);
+
+  // Add a typed visit, so it will be autofilled.
+  yield PlacesTestUtils.addVisits({
+    uri: NetUtil.newURI("http://example.com/"),
+    transition: Ci.nsINavHistoryService.TRANSITION_TYPED
+  });
+
+  yield test_autocomplete({ desc: "DELETE the autofilled part should search",
+                            typed: "exam",
+                            autofilled: "example.com/",
+                            modified: "exam",
+                            keys: ["VK_DELETE"],
+                            action: "searchengine"
+                          });
+  yield test_autocomplete({ desc: "DELETE the final slash should visit",
+                            typed: "example.com",
+                            autofilled: "example.com/",
+                            modified: "example.com",
+                            keys: ["VK_DELETE"],
+                            action: "visiturl"
+                          });
+
+  yield test_autocomplete({ desc: "BACK_SPACE the autofilled part should search",
+                            typed: "exam",
+                            autofilled: "example.com/",
+                            modified: "exam",
+                            keys: ["VK_BACK_SPACE"],
+                            action: "searchengine"
+                          });
+  yield test_autocomplete({ desc: "BACK_SPACE the final slash should visit",
+                            typed: "example.com",
+                            autofilled: "example.com/",
+                            modified: "example.com",
+                            keys: ["VK_BACK_SPACE"],
+                            action: "visiturl"
+                          });
+
+  yield test_autocomplete({ desc: "DELETE the autofilled part, then BACK_SPACE, should search",
+                            typed: "exam",
+                            autofilled: "example.com/",
+                            modified: "exa",
+                            keys: ["VK_DELETE", "VK_BACK_SPACE"],
+                            action: "searchengine"
+                          });
+  yield test_autocomplete({ desc: "DELETE the final slash, then BACK_SPACE, should search",
+                            typed: "example.com",
+                            autofilled: "example.com/",
+                            modified: "example.co",
+                            keys: ["VK_DELETE", "VK_BACK_SPACE"],
+                            action: "visiturl"
+                          });
+
+  yield test_autocomplete({ desc: "BACK_SPACE the autofilled part, then BACK_SPACE, should search",
+                            typed: "exam",
+                            autofilled: "example.com/",
+                            modified: "exa",
+                            keys: ["VK_BACK_SPACE", "VK_BACK_SPACE"],
+                            action: "searchengine"
+                          });
+  yield test_autocomplete({ desc: "BACK_SPACE the final slash, then BACK_SPACE, should search",
+                            typed: "example.com",
+                            autofilled: "example.com/",
+                            modified: "example.co",
+                            keys: ["VK_BACK_SPACE", "VK_BACK_SPACE"],
+                            action: "visiturl"
+                          });
+
+  yield test_autocomplete({ desc: "BACK_SPACE after blur should search",
+                            typed: "ex",
+                            autofilled: "example.com/",
+                            modified: "e",
+                            keys: ["VK_BACK_SPACE"],
+                            action: "searchengine",
+                            onAutoFill: () => {
+                              gURLBar.blur();
+                              gURLBar.focus();
+                              gURLBar.selectionStart = 1;
+                              gURLBar.selectionEnd = 12;
+                            }
+                         });
+  yield test_autocomplete({ desc: "DELETE after blur should search",
+                            typed: "ex",
+                            autofilled: "example.com/",
+                            modified: "e",
+                            keys: ["VK_DELETE"],
+                            action: "searchengine",
+                            onAutoFill: () => {
+                              gURLBar.blur();
+                              gURLBar.focus();
+                              gURLBar.selectionStart = 1;
+                              gURLBar.selectionEnd = 12;
+                            }
+                          });
+  yield test_autocomplete({ desc: "double BACK_SPACE after blur should search",
+                            typed: "ex",
+                            autofilled: "example.com/",
+                            modified: "e",
+                            keys: ["VK_BACK_SPACE", "VK_BACK_SPACE"],
+                            action: "searchengine",
+                            onAutoFill: () => {
+                              gURLBar.blur();
+                              gURLBar.focus();
+                              gURLBar.selectionStart = 2;
+                              gURLBar.selectionEnd = 12;
+                            }
+                         });
+
+  yield PlacesTestUtils.clearHistory();
+});

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -44,8 +44,10 @@ NS_INTERFACE_MAP_END
 
 nsAutoCompleteController::nsAutoCompleteController() :
   mDefaultIndexCompleted(false),
-  mBackspaced(false),
   mPopupClosedByCompositionStart(false),
+  mProhibitAutoFill(false),
+  mUserClearedAutoFill(false),
+  mClearingAutoFillSearchesAgain(false),
   mCompositionState(eCompositionState_None),
   mSearchStatus(nsAutoCompleteController::STATUS_NONE),
   mRowCount(0),
@@ -119,7 +121,7 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
   mSearchString = newValue;
   mPlaceholderCompletionString.Truncate();
   mDefaultIndexCompleted = false;
-  mBackspaced = false;
+  mProhibitAutoFill = false;
   mSearchStatus = nsIAutoCompleteController::STATUS_NONE;
   mRowCount = 0;
   mSearchesOngoing = 0;
@@ -148,12 +150,17 @@ nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
       mSearches.AppendObject(search);
 
       // Count immediate searches.
-      uint16_t searchType = nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED;
       nsCOMPtr<nsIAutoCompleteSearchDescriptor> searchDesc =
         do_QueryInterface(search);
-      if (searchDesc && NS_SUCCEEDED(searchDesc->GetSearchType(&searchType)) &&
-          searchType == nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_IMMEDIATE)
-        mImmediateSearchesCount++;
+      if (searchDesc) {
+        uint16_t searchType = nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_DELAYED;
+        if (NS_SUCCEEDED(searchDesc->GetSearchType(&searchType)) &&
+            searchType == nsIAutoCompleteSearchDescriptor::SEARCH_TYPE_IMMEDIATE)
+          mImmediateSearchesCount++;
+
+        if (!mClearingAutoFillSearchesAgain)
+          searchDesc->GetClearingAutoFillSearchesAgain(&mClearingAutoFillSearchesAgain);
+      }
     }
   }
 
@@ -217,25 +224,39 @@ nsAutoCompleteController::HandleText()
   input->GetDisableAutoComplete(&disabled);
   NS_ENSURE_TRUE(!disabled, NS_OK);
 
-  // Don't search again if the new string is the same as the last search
+  // Usually we don't search again if the new string is the same as the last one.
   // However, if this is called immediately after compositionend event,
   // we need to search the same value again since the search was canceled
   // at compositionstart event handler.
-  if (!handlingCompositionCommit && newValue.Length() > 0 &&
+  // The new string might also be the same as the last search if the autofilled
+  // portion was cleared. In this case, we may want to search again.
+
+  bool userAddedText = newValue.Length() > mSearchString.Length();
+  mUserClearedAutoFill =
+    !userAddedText &&
+    newValue.Length() < mPlaceholderCompletionString.Length() &&
+    Substring(mPlaceholderCompletionString, 0, newValue.Length()).Equals(newValue);
+
+  bool searchAgainOnAutoFillClear = mUserClearedAutoFill && mClearingAutoFillSearchesAgain;
+
+  if (!handlingCompositionCommit &&
+      !searchAgainOnAutoFillClear &&
+      newValue.Length() > 0 &&
       newValue.Equals(mSearchString)) {
     return NS_OK;
   }
 
   // Determine if the user has removed text from the end (probably by backspacing)
-  if (newValue.Length() < mSearchString.Length() &&
-      Substring(mSearchString, 0, newValue.Length()).Equals(newValue))
-  {
+  if (!userAddedText &&
+      Substring(mSearchString, 0, newValue.Length()).Equals(newValue)) {
     // We need to throw away previous results so we don't try to search through them again
-    ClearResults();
-    mBackspaced = true;
+    if (newValue.Length() < mSearchString.Length()) {
+      ClearResults();
+    }
+    mProhibitAutoFill = true;
     mPlaceholderCompletionString.Truncate();
   } else {
-    mBackspaced = false;
+    mProhibitAutoFill = false;
   }
 
   mSearchString = newValue;
@@ -1151,6 +1172,13 @@ nsAutoCompleteController::StartSearch(uint16_t aSearchType)
     if (NS_FAILED(rv))
         return rv;
 
+    // FormFill expects the searchParam to only contain the input element id,
+    // other consumers may have other expectations, so this modifies it only
+    // for new consumers handling autoFill by themselves.
+    if (mProhibitAutoFill && mClearingAutoFillSearchesAgain) {
+      searchParam.AppendLiteral(" prohibit-autofill");
+    }
+
     rv = search->StartSearch(mSearchString, searchParam, result, static_cast<nsIAutoCompleteObserver *>(this));
     if (NS_FAILED(rv)) {
       ++mSearchesFailed;
@@ -1225,6 +1253,7 @@ nsAutoCompleteController::MaybeCompletePlaceholder()
   // In addition, the selection must be at the end of the current input to
   // trigger the placeholder completion.
   bool usePlaceholderCompletion =
+    !mUserClearedAutoFill &&
     !mPlaceholderCompletionString.IsEmpty() &&
     mPlaceholderCompletionString.Length() > mSearchString.Length() &&
     selectionEnd == selectionStart &&
@@ -1613,7 +1642,7 @@ nsAutoCompleteController::ClearResults()
 nsresult
 nsAutoCompleteController::CompleteDefaultIndex(int32_t aResultIndex)
 {
-  if (mDefaultIndexCompleted || mBackspaced || mSearchString.Length() == 0 || !mInput)
+  if (mDefaultIndexCompleted || mProhibitAutoFill || mSearchString.Length() == 0 || !mInput)
     return NS_OK;
 
   nsCOMPtr<nsIAutoCompleteInput> input(mInput);

toolkit/components/autocomplete/nsAutoCompleteController.h

@@ -139,8 +139,19 @@ protected:
   nsString mSearchString;
   nsString mPlaceholderCompletionString;
   bool mDefaultIndexCompleted;
-  bool mBackspaced;
   bool mPopupClosedByCompositionStart;
+
+  // Whether autofill is allowed for the next search. May be retrieved by the
+  // search through the "prohibit-autofill" searchParam.
+  bool mProhibitAutoFill;
+
+  // Indicates whether the user cleared the autofilled part, returning to the
+  // originally entered search string.
+  bool mUserClearedAutoFill;
+
+  // Indicates whether clearing the autofilled string should issue a new search.
+  bool mClearingAutoFillSearchesAgain;
+
   enum CompositionState {
     eCompositionState_None,
     eCompositionState_Composing,

toolkit/components/autocomplete/nsIAutoCompleteSearch.idl

@@ -50,7 +50,7 @@ interface nsIAutoCompleteObserver : nsISupports
   void onUpdateSearchResult(in nsIAutoCompleteSearch search, in nsIAutoCompleteResult result);
 };
 
-[scriptable, uuid(02314d6e-b730-40cc-a215-221554d77064)]
+[scriptable, uuid(4c3e7462-fbfb-4310-8f4b-239238392b75)]
 interface nsIAutoCompleteSearchDescriptor : nsISupports
 {
   // The search is started after the timeout specified by the corresponding
@@ -65,4 +65,10 @@ interface nsIAutoCompleteSearchDescriptor : nsISupports
    * Defaults to SEARCH_TYPE_DELAYED.
    */
   readonly attribute unsigned short searchType;
+
+  /*
+   * Whether a new search should be triggered when the user deletes the
+   * autofilled part.
+   */
+  readonly attribute boolean clearingAutoFillSearchesAgain;
 };

toolkit/components/places/UnifiedComplete.js

@@ -627,6 +627,7 @@ function Search(searchString, searchParam, autocompleteListener,
   this._enableActions = params.has("enable-actions");
   this._disablePrivateActions = params.has("disable-private-actions");
   this._inPrivateWindow = params.has("private-window");
+  this._prohibitAutoFill = params.has("prohibit-autofill");
 
   this._searchTokens =
     this.filterTokens(getUnfilteredSearchTokens(this._searchString));
@@ -1591,6 +1592,9 @@ Search.prototype = {
     if (this._searchString.length == 0)
       return false;
 
+    if (this._prohibitAutoFill)
+      return false;
+
     return true;
   },
 
@@ -1831,7 +1835,13 @@ UnifiedComplete.prototype = {
   //////////////////////////////////////////////////////////////////////////////
   //// nsIAutoCompleteSearchDescriptor
 
-  get searchType() Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE,
+  get searchType() {
+    return Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE;
+  },
+
+  get clearingAutoFillSearchesAgain() {
+    return true;
+  },
 
   //////////////////////////////////////////////////////////////////////////////
   //// nsISupports

toolkit/components/places/nsPlacesAutoComplete.js

@@ -1658,7 +1658,14 @@ urlInlineComplete.prototype = {
 
   //////////////////////////////////////////////////////////////////////////////
   //// nsIAutoCompleteSearchDescriptor
-  get searchType() Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE,
+
+  get searchType() {
+    return Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE;
+  },
+
+  get clearingAutoFillSearchesAgain() {
+    return false;
+  },
 
   //////////////////////////////////////////////////////////////////////////////
   //// nsIObserver