Bug 950797 - Use result from last completion in nsAutoCompleteController to reduce autoFill UI flickering until search results come back. r=mak
authorJulian Viereck <julian.viereck@gmail.com>
Tue, 30 Sep 2014 04:14:49 -0700
changeset 231238 a98aca20c7be9684db5611d46407417a5c2d9a82
parent 231237 f12535b53f5222b274673d866fdfc1970e1d4575
child 231239 95aefa6ce37fa4309806be1ccabe4048185c2efb
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs950797
milestone35.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 950797 - Use result from last completion in nsAutoCompleteController to reduce autoFill UI flickering until search results come back. r=mak
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/components/autocomplete/nsAutoCompleteController.h
toolkit/content/tests/chrome/chrome.ini
toolkit/content/tests/chrome/test_autocomplete_placehold_last_complete.xul
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -111,16 +111,17 @@ nsAutoCompleteController::SetInput(nsIAu
   nsAutoString newValue;
   aInput->GetTextValue(newValue);
 
   // Clear out this reference in case the new input's popup has no tree
   mTree = nullptr;
 
   // Reset all search state members to default values
   mSearchString = newValue;
+  mPlaceholderCompletionString.Truncate();
   mDefaultIndexCompleted = false;
   mBackspaced = false;
   mSearchStatus = nsIAutoCompleteController::STATUS_NONE;
   mRowCount = 0;
   mSearchesOngoing = 0;
 
   // Initialize our list of search objects
   uint32_t searchCount;
@@ -225,18 +226,20 @@ nsAutoCompleteController::HandleText()
 
   // 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))
   {
     // We need to throw away previous results so we don't try to search through them again
     ClearResults();
     mBackspaced = true;
-  } else
+    mPlaceholderCompletionString.Truncate();
+  } else {
     mBackspaced = false;
+  }
 
   mSearchString = newValue;
 
   // Don't search if the value is empty
   if (newValue.Length() == 0) {
     // If autocomplete popup was closed by compositionstart event handler,
     // we should reopen it forcibly even if the value is empty.
     if (popupClosedByCompositionStart && handlingCompositionCommit) {
@@ -1106,25 +1109,69 @@ nsAutoCompleteController::StopSearch()
     mSearchesOngoing = 0;
     // since we were searching, but now we've stopped,
     // we need to call PostSearchCleanup()
     PostSearchCleanup();
   }
   return NS_OK;
 }
 
+void
+nsAutoCompleteController::MaybeCompletePlaceholder()
+{
+  MOZ_ASSERT(mInput);
+
+  if (!mInput) { // or mInput depending on what you choose
+    MOZ_ASSERT_UNREACHABLE("Input should always be valid at this point");
+    return;
+  }
+
+  int32_t selectionStart;
+  mInput->GetSelectionStart(&selectionStart);
+  int32_t selectionEnd;
+  mInput->GetSelectionEnd(&selectionEnd);
+
+  // Check if the current input should be completed with the placeholder string
+  // from the last completion until the actual search results come back.
+  // The new input string needs to be compatible with the last completed string.
+  // E.g. if the new value is "fob", but the last completion was "foobar",
+  // then the last completion is incompatible.
+  // If the search string is the same as the last completion value, then don't
+  // complete the value again (this prevents completion to happen e.g. if the
+  // cursor is moved and StartSeaches() is invoked).
+  // In addition, the selection must be at the end of the current input to
+  // trigger the placeholder completion.
+  bool usePlaceholderCompletion =
+    !mPlaceholderCompletionString.IsEmpty() &&
+    mPlaceholderCompletionString.Length() > mSearchString.Length() &&
+    selectionEnd == selectionStart &&
+    selectionEnd == (int32_t)mSearchString.Length() &&
+    StringBeginsWith(mPlaceholderCompletionString, mSearchString,
+                    nsCaseInsensitiveStringComparator());
+
+  if (usePlaceholderCompletion) {
+    CompleteValue(mPlaceholderCompletionString);
+  } else {
+    mPlaceholderCompletionString.Truncate();
+  }
+}
+
 nsresult
 nsAutoCompleteController::StartSearches()
 {
   // Don't create a new search timer if we're already waiting for one to fire.
   // If we don't check for this, we won't be able to cancel the original timer
   // and may crash when it fires (bug 236659).
   if (mTimer || !mInput)
     return NS_OK;
 
+  // Check if the current input should be completed with the placeholder string
+  // from the last completion until the actual search results come back.
+  MaybeCompletePlaceholder();
+
   nsCOMPtr<nsIAutoCompleteInput> input(mInput);
 
   // Get the timeout for delayed searches.
   uint32_t timeout;
   input->GetTimeout(&timeout);
 
   uint32_t immediateSearchesCount = mImmediateSearchesCount;
   if (timeout == 0) {
@@ -1452,20 +1499,28 @@ nsAutoCompleteController::CompleteDefaul
 
   nsCOMPtr<nsIAutoCompleteInput> input(mInput);
 
   int32_t selectionStart;
   input->GetSelectionStart(&selectionStart);
   int32_t selectionEnd;
   input->GetSelectionEnd(&selectionEnd);
 
+  bool isPlaceholderSelected =
+      selectionEnd == (int32_t)mPlaceholderCompletionString.Length() &&
+      selectionStart == (int32_t)mSearchString.Length() &&
+      StringBeginsWith(mPlaceholderCompletionString,
+        mSearchString, nsCaseInsensitiveStringComparator());
+
   // Don't try to automatically complete to the first result if there's already
-  // a selection or the cursor isn't at the end of the input
-  if (selectionEnd != selectionStart ||
-      selectionEnd != (int32_t)mSearchString.Length())
+  // a selection or the cursor isn't at the end of the input. In case the
+  // selection is from the current placeholder completion value, then still
+  // automatically complete.
+  if (!isPlaceholderSelected && (selectionEnd != selectionStart ||
+        selectionEnd != (int32_t)mSearchString.Length()))
     return NS_OK;
 
   bool shouldComplete;
   input->GetCompleteDefaultIndex(&shouldComplete);
   if (!shouldComplete)
     return NS_OK;
 
   nsAutoString resultValue;
@@ -1598,16 +1653,17 @@ nsAutoCompleteController::CompleteValue(
   int32_t endSelect = aValue.Length();  // By default, select all of aValue.
 
   if (aValue.IsEmpty() ||
       StringBeginsWith(aValue, mSearchString,
                        nsCaseInsensitiveStringComparator())) {
     // aValue is empty (we were asked to clear mInput), or mSearchString
     // matches the beginning of aValue.  In either case we can simply
     // autocomplete to aValue.
+    mPlaceholderCompletionString = aValue;
     input->SetTextValue(aValue);
   } else {
     nsresult rv;
     nsCOMPtr<nsIIOService> ios = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
     nsAutoCString scheme;
     if (NS_SUCCEEDED(ios->ExtractScheme(NS_ConvertUTF16toUTF8(aValue), scheme))) {
       // Trying to autocomplete a URI from somewhere other than the beginning.
@@ -1618,28 +1674,31 @@ nsAutoCompleteController::CompleteValue(
 
       if ((endSelect < findIndex + mSearchStringLength) ||
           !scheme.LowerCaseEqualsLiteral("http") ||
           !Substring(aValue, findIndex, mSearchStringLength).Equals(
             mSearchString, nsCaseInsensitiveStringComparator())) {
         return NS_OK;
       }
 
-      input->SetTextValue(mSearchString +
-                          Substring(aValue, mSearchStringLength + findIndex,
-                                    endSelect));
+      mPlaceholderCompletionString = mSearchString +
+        Substring(aValue, mSearchStringLength + findIndex, endSelect);
+      input->SetTextValue(mPlaceholderCompletionString);
 
       endSelect -= findIndex; // We're skipping this many characters of aValue.
     } else {
       // Autocompleting something other than a URI from the middle.
       // Use the format "searchstring >> full string" to indicate to the user
       // what we are going to replace their search string with.
       input->SetTextValue(mSearchString + NS_LITERAL_STRING(" >> ") + aValue);
 
       endSelect = mSearchString.Length() + 4 + aValue.Length();
+
+      // Reset the last search completion.
+      mPlaceholderCompletionString.Truncate();
     }
   }
 
   input->SelectTextRange(mSearchStringLength, endSelect);
 
   return NS_OK;
 }
 
--- a/toolkit/components/autocomplete/nsAutoCompleteController.h
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.h
@@ -43,16 +43,17 @@ protected:
   nsresult ClosePopup();
 
   nsresult StartSearch(uint16_t aSearchType);
 
   nsresult BeforeSearches();
   nsresult StartSearches();
   void AfterSearches();
   nsresult ClearSearchTimer();
+  void MaybeCompletePlaceholder();
 
   nsresult ProcessResult(int32_t aSearchIndex, nsIAutoCompleteResult *aResult);
   nsresult PostSearchCleanup();
 
   nsresult EnterMatch(bool aIsPopupSelection);
   nsresult RevertTextValue();
 
   nsresult CompleteDefaultIndex(int32_t aResultIndex);
@@ -129,16 +130,17 @@ protected:
   // since otherwise the first search clears mResults.
   nsCOMArray<nsIAutoCompleteResult> mResultCache;
 
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsITreeSelection> mSelection;
   nsCOMPtr<nsITreeBoxObject> mTree;
 
   nsString mSearchString;
+  nsString mPlaceholderCompletionString;
   bool mDefaultIndexCompleted;
   bool mBackspaced;
   bool mPopupClosedByCompositionStart;
   enum CompositionState {
     eCompositionState_None,
     eCompositionState_Composing,
     eCompositionState_Committing
   };
--- a/toolkit/content/tests/chrome/chrome.ini
+++ b/toolkit/content/tests/chrome/chrome.ini
@@ -50,16 +50,17 @@ support-files =
 [test_arrowpanel.xul]
 [test_autocomplete2.xul]
 [test_autocomplete3.xul]
 [test_autocomplete4.xul]
 [test_autocomplete5.xul]
 [test_autocomplete_delayOnPaste.xul]
 [test_autocomplete_with_composition_on_input.html]
 [test_autocomplete_with_composition_on_textbox.xul]
+[test_autocomplete_placehold_last_complete.xul]
 [test_browser_drop.xul]
 skip-if = buildapp == 'mulet'
 [test_bug253481.xul]
 [test_bug263683.xul]
 [test_bug304188.xul]
 [test_bug331215.xul]
 [test_bug360220.xul]
 [test_bug360437.xul]
new file mode 100644
--- /dev/null
+++ b/toolkit/content/tests/chrome/test_autocomplete_placehold_last_complete.xul
@@ -0,0 +1,309 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+
+<window title="Autocomplete Widget Test"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="runTest();">
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
+  <script type="application/javascript"
+          src="chrome://global/content/globalOverlay.js"/>
+
+<textbox id="autocomplete"
+         type="autocomplete"
+         completedefaultindex="true"
+         timeout="0"
+         autocompletesearch="simple"/>
+
+<script class="testbody" type="application/javascript">
+<![CDATA[
+
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+function autoCompleteSimpleResult(aString, searchId) {
+  this.searchString = aString;
+  this.searchResult = Components.interfaces.nsIAutoCompleteResult.RESULT_SUCCESS;
+  this.matchCount = 1;
+  if (aString.startsWith('ret')) {
+    this._param = autoCompleteSimpleResult.retireCompletion;
+  } else {
+    this._param = "Result";
+  }
+  this._searchId = searchId;
+}
+autoCompleteSimpleResult.retireCompletion = "Retire";
+autoCompleteSimpleResult.prototype = {
+  _param: "",
+  searchString: null,
+  searchResult: Components.interfaces.nsIAutoCompleteResult.RESULT_FAILURE,
+  defaultIndex: 0,
+  errorDescription: null,
+  matchCount: 0,
+  getValueAt: function() { return this._param; },
+  getCommentAt: function() { return null; },
+  getStyleAt: function() { return null; },
+  getImageAt: function() { return null; },
+  getLabelAt: function() { return null; },
+  removeValueAt: function() {}
+};
+
+var searchCounter = 0;
+
+// A basic autocomplete implementation that returns one result.
+let autoCompleteSimple = {
+  classID: Components.ID("0a2afbdb-f30e-47d1-9cb1-0cd160240aca"),
+  contractID: "@mozilla.org/autocomplete/search;1?name=simple",
+  searchAsync: false,
+  pendingSearch: null,
+
+  QueryInterface: XPCOMUtils.generateQI([
+    Components.interfaces.nsIFactory,
+    Components.interfaces.nsIAutoCompleteSearch
+  ]),
+  createInstance: function (outer, iid) {
+    return this.QueryInterface(iid);
+  },
+
+  registerFactory: function () {
+    let registrar =
+      Components.manager.QueryInterface(Components.interfaces.nsIComponentRegistrar);
+    registrar.registerFactory(this.classID, "Test Simple Autocomplete",
+                              this.contractID, this);
+  },
+  unregisterFactory: function () {
+    let registrar =
+      Components.manager.QueryInterface(Components.interfaces.nsIComponentRegistrar);
+    registrar.unregisterFactory(this.classID, this);
+  },
+
+  startSearch: function (aString, aParam, aResult, aListener) {
+    let result = new autoCompleteSimpleResult(aString);
+
+    if (this.searchAsync) {
+      // Simulate an async search by using a timeout before invoking the
+      // |onSearchResult| callback.
+      // Store the searchTimeout such that it can be canceled if stopSearch is called.
+      this.pendingSearch = setTimeout(() => {
+        this.pendingSearch = null;
+
+        aListener.onSearchResult(this, result);
+
+         // Move to the next step in the async test.
+        asyncTest.next();
+      }, 0);
+    } else {
+      aListener.onSearchResult(this, result);
+    }
+  },
+  stopSearch: function () {
+    clearTimeout(this.pendingSearch);
+  }
+};
+
+SimpleTest.waitForExplicitFinish();
+
+// XPFE AutoComplete needs to register early.
+autoCompleteSimple.registerFactory();
+
+let gACTimer;
+let gAutoComplete;
+let asyncTest;
+
+let searchCompleteTimeoutId = null;
+
+function finishTest() {
+  // Unregister the factory so that we don't get in the way of other tests
+  autoCompleteSimple.unregisterFactory();
+  SimpleTest.finish();
+}
+
+function runTest() {
+  gAutoComplete = $("autocomplete");
+  gAutoComplete.focus();
+
+  // Return the search results synchronous, which also makes the completion
+  // happen synchronous.
+  autoCompleteSimple.searchAsync = false;
+
+  synthesizeKey("r", {});
+  is(gAutoComplete.value, "result", "Value should be autocompleted immediately");
+
+  synthesizeKey("e", {});
+  is(gAutoComplete.value, "result", "Value should be autocompleted immediately");
+
+  synthesizeKey("VK_DELETE", {});
+  is(gAutoComplete.value, "re", "Deletion should not complete value");
+
+  synthesizeKey("VK_BACK_SPACE", {});
+  is(gAutoComplete.value, "r", "Backspace should not complete value");
+
+  synthesizeKey("VK_LEFT", {});
+  is(gAutoComplete.value, "r", "Value should stay same when navigating with cursor");
+
+  runAsyncTest();
+}
+
+function* asyncTestGenerator() {
+  synthesizeKey("r", {});
+  synthesizeKey("e", {});
+  is(gAutoComplete.value, "re", "Value should not be autocompleted immediately");
+
+  // Calling |yield undefined| makes this generator function wait until
+  // |asyncTest.next();| is called. This happens from within the
+  // |autoCompleteSimple.startSearch()| function once the simulated async
+  // search has finished.
+  // Therefore, the effect of the |yield undefined;| here (and the ones) below
+  // is to wait until the async search result comes back.
+  yield undefined;
+
+  is(gAutoComplete.value, "result", "Value should be autocompleted");
+
+  // Test if typing the `s` character completes directly based on the last
+  // completion
+  synthesizeKey("s", {});
+  is(gAutoComplete.value, "result", "Value should be completed immediately");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "result", "Value should be autocompleted to same value");
+  synthesizeKey("VK_DELETE", {});
+  is(gAutoComplete.value, "res", "Deletion should not complete value");
+
+  // No |yield undefined| needed here as no completion is triggered by the deletion.
+
+  is(gAutoComplete.value, "res", "Still no complete value after deletion");
+
+  synthesizeKey("VK_BACK_SPACE", {});
+  is(gAutoComplete.value, "re", "Backspace should not complete value");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "re", "Value after search due to backspace should stay the same"); (3)
+
+  // Typing a character that is not like the previous match. In this case, the
+  // completion cannot happen directly and therefore the value will be completed
+  // only after the search has finished.
+  synthesizeKey("t", {});
+  is(gAutoComplete.value, "ret", "Value should not be autocompleted immediately");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted");
+
+  synthesizeKey("i", {});
+  is(gAutoComplete.value, "retire", "Value should be autocompleted immediately");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted to the same value");
+
+  // Setup the scene to test how the completion behaves once the placeholder
+  // completion and the result from the search do not agree with each other.
+  gAutoComplete.value = 'r';
+  // Need to type two characters as the input was reset and the autocomplete
+  // controller things, ther user hit the backspace button, in which case
+  // no completion is performed. But as a completion is desired, another
+  // character `t` is typed afterwards.
+  synthesizeKey("e", {});
+  yield undefined;
+  synthesizeKey("t", {});
+  is(gAutoComplete.value, "ret", "Value should not be autocompleted");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted");
+
+  // The placeholder string is now set to "retire". Changing the completion
+  // string to "retirement" and see what the completion will turn out like.
+  autoCompleteSimpleResult.retireCompletion = "Retirement";
+  synthesizeKey("i", {});
+  is(gAutoComplete.value, "retire", "Value should be autocompleted based on placeholder");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retirement", "Value should be autocompleted based on search result");
+
+  // Change the search result to `Retire` again and see if the new result is
+  // complited.
+  autoCompleteSimpleResult.retireCompletion = "Retire";
+  synthesizeKey("r", {});
+  is(gAutoComplete.value, "retirement", "Value should be autocompleted based on placeholder");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted based on search result");
+
+  // Complete the value
+  gAutoComplete.value = 're';
+  synthesizeKey("t", {});
+  yield undefined;
+  synthesizeKey("i", {});
+  is(gAutoComplete.value, "reti", "Value should not be autocompleted");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted");
+
+  // Remove the selected text "re" (1) and the "et" (2). Afterwards, add it again (3).
+  // This should not cause the completion to kick in.
+  synthesizeKey("VK_DELETE", {}); // (1)
+
+  is(gAutoComplete.value, "reti", "Value should not complete after deletion");
+
+  gAutoComplete.selectionStart = 1;
+  gAutoComplete.selectionEnd = 3;
+  synthesizeKey("VK_DELETE", {}); // (2)
+
+  is(gAutoComplete.value, "ri", "Value should stay unchanged after removing character in the middle");
+
+  yield undefined;
+
+  synthesizeKey("e", {}); // (3.1)
+  is(gAutoComplete.value, "rei", "Inserting a character in the middle should not complete the value");
+
+  yield undefined;
+
+  synthesizeKey("t", {}); // (3.2)
+  is(gAutoComplete.value, "reti", "Inserting a character in the middle should not complete the value");
+
+  yield undefined;
+
+  // Adding a new character at the end should not cause the completion to happen again
+  // as the completion failed before.
+  gAutoComplete.selectionStart = 4;
+  gAutoComplete.selectionEnd = 4;
+  synthesizeKey("r", {});
+  is(gAutoComplete.value, "retir", "Value should not be autocompleted immediately");
+
+  yield undefined;
+
+  is(gAutoComplete.value, "retire", "Value should be autocompleted");
+
+  finishTest();
+  yield undefined;
+}
+
+function runAsyncTest() {
+  gAutoComplete.value = '';
+  autoCompleteSimple.searchAsync = true;
+
+  asyncTest = asyncTestGenerator();
+  asyncTest.next();
+}
+]]>
+</script>
+
+<body xmlns="http://www.w3.org/1999/xhtml">
+<p id="display">
+</p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+</pre>
+</body>
+
+</window>