Bug 1301093 - Part 1: don't wait for a result when we won't get one. r=adw draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 07 Sep 2016 17:44:46 +0200
changeset 413738 39144a190f27257b190178308b26fa3f7bea15a2
parent 413613 8a494adbc5cced90a4edf0c98cffde906bf7f3ae
child 413739 b1175292d466c9d00f04ff57bfbe0522c2f7e062
push id29482
push usermak77@bonardo.net
push dateWed, 14 Sep 2016 18:58:13 +0000
reviewersadw
bugs1301093
milestone51.0a1
Bug 1301093 - Part 1: don't wait for a result when we won't get one. r=adw MozReview-Commit-ID: E6l9mW3ZoKp
browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
browser/base/content/urlbarBindings.xml
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/components/autocomplete/nsIAutoCompleteController.idl
toolkit/components/satchel/nsFormFillController.cpp
--- a/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
+++ b/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
@@ -1,23 +1,43 @@
-add_task(function*() {
-  registerCleanupFunction(function* () {
-    yield PlacesUtils.bookmarks.remove(bm);
-  });
+// The order of these tests matters!
 
+add_task(function* setup () {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
   let bm = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: "http://example.com/?q=%s",
                                                 title: "test" });
+  registerCleanupFunction(function* () {
+    yield PlacesUtils.bookmarks.remove(bm);
+    yield BrowserTestUtils.removeTab(tab);
+  });
   yield PlacesUtils.keywords.insert({ keyword: "keyword",
                                       url: "http://example.com/?q=%s" });
+  // Needs at least one success.
+  ok(true, "Setup complete");
+});
 
-  yield new Promise(resolve => waitForFocus(resolve, window));
-
+add_task(function* test_keyword() {
   yield promiseAutocompleteResultPopup("keyword bear");
   gURLBar.focus();
   EventUtils.synthesizeKey("d", {});
   EventUtils.synthesizeKey("VK_RETURN", {});
+  info("wait for the page to load");
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
+                                      false, "http://example.com/?q=beard");
+});
 
-  yield promiseTabLoadEvent(gBrowser.selectedTab);
-  is(gBrowser.selectedBrowser.currentURI.spec,
-     "http://example.com/?q=beard",
-     "Latest typed characters should have been used");
+add_task(function* test_sametext() {
+  yield promiseAutocompleteResultPopup("example.com", window, true);
+
+  // Simulate re-entering the same text searched the last time. This may happen
+  // through a copy paste, but clipboard handling is not much reliable, so just
+  // fire an input event.
+  info("synthesize input event");
+  let event = document.createEvent("Events");
+  event.initEvent("input", true, true);
+  gURLBar.dispatchEvent(event);
+  EventUtils.synthesizeKey("VK_RETURN", {});
+
+  info("wait for the page to load");
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
+                                       false, "http://example.com/");
 });
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1000,18 +1000,22 @@ file, You can obtain one at http://mozil
 
       <method name="onInput">
         <parameter name="aEvent"/>
         <body><![CDATA[
           if (!this.mIgnoreInput && this.mController.input == this) {
             this._value = this.inputField.value;
             gBrowser.userTypedValue = this.value;
             this.valueIsTyped = true;
-            this.gotResultForCurrentQuery = false;
-            this.mController.handleText();
+            // Only wait for a result when we are sure to get one.  In some
+            // cases, like when pasting the same exact text, we may not fire
+            // a new search and we won't get a result.
+            if (this.mController.handleText()) {
+              this.gotResultForCurrentQuery = false;
+            }
           }
           this.resetActionType();
         ]]></body>
       </method>
 
       <method name="handleEnter">
         <parameter name="event"/>
         <body><![CDATA[
@@ -1048,17 +1052,18 @@ file, You can obtain one at http://mozil
       <method name="handleDelete">
         <body><![CDATA[
           // If the heuristic result is selected, then the autocomplete
           // controller's handleDelete implementation will remove it, which is
           // not what we want.  So in that case, call handleText so it acts as
           // a backspace on the text value instead of removing the result.
           if (this.popup.selectedIndex == 0 &&
               this.popup._isFirstResultHeuristic) {
-            return this.mController.handleText();
+            this.mController.handleText();
+            return false;
           }
           return this.mController.handleDelete();
         ]]></body>
       </method>
 
       <field name="_userMadeSearchSuggestionsChoice"><![CDATA[
         false
       ]]></field>
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -175,18 +175,19 @@ NS_IMETHODIMP
 nsAutoCompleteController::StartSearch(const nsAString &aSearchString)
 {
   mSearchString = aSearchString;
   StartSearches();
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsAutoCompleteController::HandleText()
+nsAutoCompleteController::HandleText(bool *_retval)
 {
+  *_retval = false;
   // Note: the events occur in the following order when IME is used.
   // 1. a compositionstart event(HandleStartComposition)
   // 2. some input events (HandleText), eCompositionState_Composing
   // 3. a compositionend event(HandleEndComposition)
   // 4. an input event(HandleText), eCompositionState_Committing
   // We should do nothing during composition.
   if (mCompositionState == eCompositionState_Composing) {
     return NS_OK;
@@ -279,16 +280,17 @@ nsAutoCompleteController::HandleText()
       bool cancel;
       HandleKeyNavigation(nsIDOMKeyEvent::DOM_VK_DOWN, &cancel);
       return NS_OK;
     }
     ClosePopup();
     return NS_OK;
   }
 
+  *_retval = true;
   StartSearches();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAutoCompleteController::HandleEnter(bool aIsPopupSelection,
                                       nsIDOMEvent *aEvent,
@@ -618,28 +620,30 @@ nsAutoCompleteController::HandleDelete(b
   if (!mInput)
     return NS_OK;
 
   nsCOMPtr<nsIAutoCompleteInput> input(mInput);
   bool isOpen = false;
   input->GetPopupOpen(&isOpen);
   if (!isOpen || mRowCount <= 0) {
     // Nothing left to delete, proceed as normal
-    HandleText();
+    bool unused = false;
+    HandleText(&unused);
     return NS_OK;
   }
 
   nsCOMPtr<nsIAutoCompletePopup> popup;
   input->GetPopup(getter_AddRefs(popup));
 
   int32_t index, searchIndex, rowIndex;
   popup->GetSelectedIndex(&index);
   if (index == -1) {
     // No row is selected in the list
-    HandleText();
+    bool unused = false;
+    HandleText(&unused);
     return NS_OK;
   }
 
   RowIndexToSearch(index, &searchIndex, &rowIndex);
   NS_ENSURE_TRUE(searchIndex >= 0 && rowIndex >= 0, NS_ERROR_FAILURE);
 
   nsIAutoCompleteResult *result = mResults.SafeObjectAt(searchIndex);
   NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
@@ -1188,17 +1192,17 @@ nsAutoCompleteController::StartSearch(ui
           searchResult != nsIAutoCompleteResult::RESULT_SUCCESS_ONGOING &&
           searchResult != nsIAutoCompleteResult::RESULT_NOMATCH)
         result = nullptr;
     }
 
     nsAutoString searchParam;
     nsresult rv = input->GetSearchParam(searchParam);
     if (NS_FAILED(rv))
-        return 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");
     }
 
--- a/toolkit/components/autocomplete/nsIAutoCompleteController.idl
+++ b/toolkit/components/autocomplete/nsIAutoCompleteController.idl
@@ -33,57 +33,61 @@ interface nsIAutoCompleteController : ns
    */
   readonly attribute unsigned long matchCount;
 
   /*
    * Start a search on a string, assuming the input property is already set.
    */
   void startSearch(in AString searchString);
 
-  /* 
+  /*
    * Stop all asynchronous searches
    */
   void stopSearch();
 
   /*
    * Notify the controller that the user has changed text in the textbox.
    * This includes all means of changing the text value, including typing a
    * character, backspacing, deleting, pasting, committing composition or
    * canceling composition.
    *
    * NOTE: handleText() must be called after composition actually ends, even if
    *       the composition is canceled and the textbox value isn't changed.
    *       Then, implementation of handleText() can access the editor when
    *       it's not in composing mode. DOM compositionend event is not good
    *       timing for calling handleText(). DOM input event immediately after
    *       DOM compositionend event is the best timing to call this.
+   *
+   * @return whether this handler started a new search.
    */
-  void handleText();
+  boolean handleText();
 
   /*
    * Notify the controller that the user wishes to enter the current text. If
    * aIsPopupSelection is true, then a selection was made from the popup, so
    * fill this value into the input field before continuing. If false, just
    * use the current value of the input field.
    *
    * @param aIsPopupSelection
    *        Pass true if the selection was made from the popup.
    * @param aEvent
    *        The event that triggered the enter, like a key event if the user
    *        pressed the Return key or a click event if the user clicked a popup
    *        item.
-   * @return True if the controller wishes to prevent event propagation and default event
+   * @return Whether the controller wishes to prevent event propagation and
+   *         default event.
    */
   boolean handleEnter(in boolean aIsPopupSelection,
                       [optional] in nsIDOMEvent aEvent);
 
   /*
    * Notify the controller that the user wishes to revert autocomplete
    *
-   * @return True if the controller wishes to prevent event propagation and default event
+   * @return Whether the controller wishes to prevent event propagation and
+   *         default event.
    */
   boolean handleEscape();
 
   /*
    * Notify the controller that the user wishes to start composition
    *
    * NOTE: nsIAutoCompleteController implementation expects that this is called
    *       by DOM compositionstart handler.
@@ -93,35 +97,38 @@ interface nsIAutoCompleteController : ns
   /*
    * Notify the controller that the user wishes to end composition
    *
    * NOTE: nsIAutoCompleteController implementation expects that this is called
    *       by DOM compositionend handler.
    */
   void handleEndComposition();
 
-  /* 
+  /*
    * Handle tab. Just closes up.
    */
   void handleTab();
 
   /*
    * Notify the controller of the following key navigation events:
    *   up, down, left, right, page up, page down
    *
-   * @return True if the controller wishes to prevent event propagation and default event
+   * @return Whether the controller wishes to prevent event propagation and
+   *         default event
    */
   boolean handleKeyNavigation(in unsigned long key);
 
   /*
    * Notify the controller that the user chose to delete the current
    * auto-complete result.
+   *
+   * @return Whether the controller removed a result item.
    */
   boolean handleDelete();
-  
+
   /*
    * Get the value of the result at a given index in the last completed search
    */
   AString getValueAt(in long index);
 
   /*
    * Get the label of the result at a given index in the last completed search
    */
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -820,18 +820,19 @@ nsFormFillController::HandleEvent(nsIDOM
   }
   if (type.EqualsLiteral("mousedown")) {
     return MouseDown(aEvent);
   }
   if (type.EqualsLiteral("keypress")) {
     return KeyPress(aEvent);
   }
   if (type.EqualsLiteral("input")) {
+    bool unused = false;
     return (!mSuppressOnInput && mController && mFocusedInput) ?
-           mController->HandleText() : NS_OK;
+           mController->HandleText(&unused) : NS_OK;
   }
   if (type.EqualsLiteral("blur")) {
     if (mFocusedInput)
       StopControllingInput();
     return NS_OK;
   }
   if (type.EqualsLiteral("compositionstart")) {
     NS_ASSERTION(mController, "should have a controller!");
@@ -931,37 +932,39 @@ nsFormFillController::KeyPress(nsIDOMEve
   if (!mFocusedInput || !mController)
     return NS_OK;
 
   nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aEvent);
   if (!keyEvent)
     return NS_ERROR_FAILURE;
 
   bool cancel = false;
+  bool unused = false;
 
   uint32_t k;
   keyEvent->GetKeyCode(&k);
   switch (k) {
   case nsIDOMKeyEvent::DOM_VK_DELETE:
 #ifndef XP_MACOSX
     mController->HandleDelete(&cancel);
     break;
   case nsIDOMKeyEvent::DOM_VK_BACK_SPACE:
-    mController->HandleText();
+    mController->HandleText(&unused);
     break;
 #else
   case nsIDOMKeyEvent::DOM_VK_BACK_SPACE:
     {
       bool isShift = false;
       keyEvent->GetShiftKey(&isShift);
 
-      if (isShift)
+      if (isShift) {
         mController->HandleDelete(&cancel);
-      else
-        mController->HandleText();
+      } else {
+        mController->HandleText(&unused);
+      }
 
       break;
     }
 #endif
   case nsIDOMKeyEvent::DOM_VK_PAGE_UP:
   case nsIDOMKeyEvent::DOM_VK_PAGE_DOWN:
     {
       bool isCtrl, isAlt, isMeta;
@@ -1061,17 +1064,18 @@ nsFormFillController::MouseDown(nsIDOMEv
   if (!input)
     return NS_OK;
 
   nsAutoString value;
   input->GetTextValue(value);
   if (value.Length() > 0) {
     // Show the popup with a filtered result set
     mController->SetSearchString(EmptyString());
-    mController->HandleText();
+    bool unused = false;
+    mController->HandleText(&unused);
   } else {
     // Show the popup with the complete result set.  Can't use HandleText()
     // because it doesn't display the popup if the input is blank.
     bool cancel = false;
     mController->HandleKeyNavigation(nsIDOMKeyEvent::DOM_VK_DOWN, &cancel);
   }
 
   return NS_OK;