Bug 325842 Make setting the autocomplete attribute 'completeDefaultIndex' do something sane when the search string does not match the beginning of the result string [r=enndeakin ui-r=mconnor]

toolkit/components/autocomplete/src/nsAutoCompleteController.cpp

@@ -1388,8 +1388,6 @@ nsAutoCompleteController::CompleteValue(nsString &aValue,
     // autocomplete to aValue.
     mInput->SetTextValue(aValue);
   } else {
-    PRInt32 findIndex;  // Offset of mSearchString within aValue.
-
     nsresult rv;
     nsCOMPtr<nsIIOService> ios = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1399,7 +1397,7 @@ nsAutoCompleteController::CompleteValue(nsString &aValue,
       // Only succeed if the missing portion is "http://"; otherwise do not
       // autocomplete.  This prevents us from "helpfully" autocompleting to a
       // URI that isn't equivalent to what the user expected.
-      findIndex = 7; // length of "http://"
+      const PRInt32 findIndex = 7; // length of "http://"
 
       if ((endSelect < findIndex + mSearchStringLength) ||
           !scheme.LowerCaseEqualsLiteral("http") ||
@@ -1407,27 +1405,20 @@ nsAutoCompleteController::CompleteValue(nsString &aValue,
             mSearchString, nsCaseInsensitiveStringComparator())) {
         return NS_OK;
       }
+
+      mInput->SetTextValue(mSearchString +
+                           Substring(aValue, mSearchStringLength + findIndex,
+                                     endSelect));
+
+      endSelect -= findIndex; // We're skipping this many characters of aValue.
     } else {
-      // Autocompleting something other than a URI from the middle.  Assume we
-      // can just go ahead and autocomplete the missing final portion; this
-      // seems like a problematic assumption...
-      nsString::const_iterator iter, end;
-      aValue.BeginReading(iter);
-      aValue.EndReading(end);
-      const nsString::const_iterator::pointer start = iter.get();
-      ++iter;  // Skip past beginning since we know that doesn't match
+      // 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.
+      mInput->SetTextValue(mSearchString + NS_LITERAL_STRING(" >> ") + aValue);
 
-      FindInReadable(mSearchString, iter, end,
-                     nsCaseInsensitiveStringComparator());
-
-      findIndex = iter.get() - start;
+      endSelect = mSearchString.Length() + 4 + aValue.Length();
     }
-
-    mInput->SetTextValue(mSearchString +
-                         Substring(aValue, mSearchStringLength + findIndex,
-                                   endSelect));
-
-    endSelect -= findIndex; // We're skipping this many characters of aValue.
   }
 
   mInput->SelectTextRange(selectDifference ?

toolkit/content/tests/chrome/Makefile.in

@@ -69,6 +69,7 @@ _TEST_FILES = 	findbar_window.xul \
 		window_preferences2.xul \
 		window_preferences3.xul \
 		test_autocomplete2.xul \
+		test_autocomplete3.xul \
 		test_keys.xul \
 		window_keys.xul \
 		$(NULL)

toolkit/content/tests/chrome/test_autocomplete3.xul

@@ -0,0 +1,174 @@
+<?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 3"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <script type="application/javascript" 
+          src="chrome://mochikit/content/MochiKit/packed.js"/>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
+
+<textbox id="autocomplete" type="autocomplete"
+         autocompletesearch="simple"
+         onsearchcomplete="checkResult();"/>
+
+<script class="testbody" type="application/javascript">
+<![CDATA[
+
+// Set to indicate whether or not we want autoCompleteSimple to return a result
+var returnResult = true;
+
+const ACR = Components.interfaces.nsIAutoCompleteResult;
+
+// This result can't be constructed in-line, because otherwise we leak memory.
+function nsAutoCompleteSimpleResult(aString)
+{
+  this.searchString = aString;
+  if (returnResult) {
+    this.searchResult = ACR.RESULT_SUCCESS;
+    this.matchCount = 1;
+    this._param = "result";
+  }
+}
+
+nsAutoCompleteSimpleResult.prototype = {
+ _param: "",
+ searchString: null,
+ searchResult: ACR.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; },
+ removeValueAt: function() {}
+};
+
+// A basic autocomplete implementation that either returns one result or none
+var autoCompleteSimpleID = Components.ID("0a2afbdb-f30e-47d1-9cb1-0cd160240aca");
+var autoCompleteSimpleName = "@mozilla.org/autocomplete/search;1?name=simple"
+var autoCompleteSimple = {
+  QueryInterface: function(iid) {
+    if (iid.equals(Components.interfaces.nsISupports) ||
+        iid.equals(Components.interfaces.nsIFactory) ||
+        iid.equals(Components.interfaces.nsIAutoCompleteSearch))
+      return this;
+
+    throw Components.results.NS_ERROR_NO_INTERFACE;
+  },
+
+  createInstance: function(outer, iid) {
+    return this.QueryInterface(iid);
+  },
+
+  startSearch: function(aString, aParam, aResult, aListener) {
+    var result = new nsAutoCompleteSimpleResult(aString);
+    aListener.onSearchResult(this, result);
+  },
+
+  stopSearch: function() {}
+};
+
+var componentManager = Components.manager
+                                 .QueryInterface(Components.interfaces.nsIComponentRegistrar);
+componentManager.registerFactory(autoCompleteSimpleID, "Test Simple Autocomplete",
+                                 autoCompleteSimpleName, autoCompleteSimple);
+
+
+// Test Bug 325842 - autoFill and autoFillAfterMatch
+
+SimpleTest.waitForExplicitFinish();
+setTimeout(startTest, 0);
+
+var currentTest = 0;
+
+// Note the entries for these tests (key) are incremental.
+const tests = [
+  { completeDefaultIndex: "false", key: "r", result: "r",
+    start: 1, end: 1 },
+  { completeDefaultIndex: "true", key: "e", result: "result",
+    start: 2, end: 6 },
+  { completeDefaultIndex: "true", key: "t", result: "ret >> result",
+    start: 3, end: 13 }
+];
+
+function startTest() {
+  var autocomplete = $("autocomplete");
+
+  // These should not be set by default.
+  is(autocomplete.hasAttribute("completedefaultindex"), false,
+     "completedefaultindex not set by default");
+
+  autocomplete.completeDefaultIndex = "true";
+
+  is(autocomplete.getAttribute("completedefaultindex"), "true",
+     "completedefaultindex attribute set correctly");
+  is(autocomplete.completeDefaultIndex, true,
+     "autoFill getter returned correctly");
+
+  autocomplete.completeDefaultIndex = "false";
+
+  is(autocomplete.getAttribute("completedefaultindex"), "false",
+     "completedefaultindex attribute set to false correctly");
+  is(autocomplete.completeDefaultIndex, false,
+     "completeDefaultIndex getter returned false correctly");
+
+  checkNext();
+}
+
+function checkNext() {
+  var autocomplete = $("autocomplete");
+
+  autocomplete.completeDefaultIndex = tests[currentTest].completeDefaultIndex;
+  autocomplete.focus();
+
+  synthesizeKey(tests[currentTest].key, {});
+}
+
+function checkResult() {
+  var autocomplete = $("autocomplete");
+  var style = window.getComputedStyle(autocomplete, "");
+
+  is(autocomplete.value, tests[currentTest].result,
+     "Test " + currentTest + ": autocomplete.value should equal '" +
+     tests[currentTest].result + "'");
+
+  is(autocomplete.selectionStart, tests[currentTest].start,
+     "Test " + currentTest + ": autocomplete selection should start at " +
+     tests[currentTest].start);
+
+  is(autocomplete.selectionEnd, tests[currentTest].end,
+     "Test " + currentTest + ": autocomplete selection should end at " +
+     tests[currentTest].end);
+
+  ++currentTest;
+
+  if (currentTest < tests.length)
+    setTimeout(checkNext, 0);
+  else {
+    setTimeout(function() {
+      // Unregister the factory so that we don't get in the way of other tests
+      componentManager.unregisterFactory(autoCompleteSimpleID, autoCompleteSimple);
+      SimpleTest.finish();
+    }, 0);
+  }
+}
+
+]]>
+</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>