Bug 497541 - Autocomplete dropdown should update when text inserted before end. r=gavin

toolkit/components/autocomplete/public/nsIAutoCompleteController.idl

@@ -41,7 +41,7 @@
 
 interface nsIAutoCompleteInput;
 
-[scriptable, uuid(6F08D134-8536-4B28-B456-D150FBAA66A9)]
+[scriptable, uuid(46a86173-0ab5-44b2-ab51-722cb3db1b60)]
 interface nsIAutoCompleteController : nsISupports
 {
   /*
@@ -82,7 +82,7 @@ interface nsIAutoCompleteController : nsISupports
    * means of changing the text value, including typing a character, backspacing, deleting, or
    * pasting in an entirely new value.
    */
-  void handleText(in boolean aIgnoreSelection);
+  void handleText();
 
   /*
    * Notify the controller that the user wishes to enter the current text. If

toolkit/components/autocomplete/src/nsAutoCompleteController.cpp

@@ -194,7 +194,7 @@ nsAutoCompleteController::StartSearch(const nsAString &aSearchString)
 }
 
 NS_IMETHODIMP
-nsAutoCompleteController::HandleText(PRBool aIgnoreSelection)
+nsAutoCompleteController::HandleText()
 {
   if (!mInput) {
     // Stop all searches in case they are async.
@@ -254,12 +254,6 @@ nsAutoCompleteController::HandleText(PRBool aIgnoreSelection)
   } else
     mBackspaced = PR_FALSE;
 
-  if (mRowCount == 0)
-    // XXX Handle the case where we have no results because of an ignored prefix.
-    // This is just a hack. I have no idea what I'm doing. Hewitt, fix this the right
-    // way when you get a chance. -dwh
-    ClearResults();
-
   mSearchString = newValue;
 
   // Don't search if the value is empty
@@ -268,18 +262,7 @@ nsAutoCompleteController::HandleText(PRBool aIgnoreSelection)
     return NS_OK;
   }
 
-  if (aIgnoreSelection) {
-    StartSearchTimer();
-  } else {
-    // Kick off the search only if the cursor is at the end of the textbox
-  PRInt32 selectionStart;
-  input->GetSelectionStart(&selectionStart);
-  PRInt32 selectionEnd;
-  input->GetSelectionEnd(&selectionEnd);
-
-  if (selectionStart == selectionEnd && selectionStart == (PRInt32) mSearchString.Length())
-    StartSearchTimer();
-  }
+  StartSearchTimer();
 
   return NS_OK;
 }
@@ -379,7 +362,7 @@ nsAutoCompleteController::HandleEndComposition()
   SetSearchString(EmptyString());
   if (!value.IsEmpty()) {
     // Show the popup with a filtered result set
-    HandleText(PR_TRUE);
+    HandleText();
   } else if (forceOpenPopup) {
     PRBool cancel;
     HandleKeyNavigation(nsIDOMKeyEvent::DOM_VK_DOWN, &cancel);
@@ -561,7 +544,7 @@ nsAutoCompleteController::HandleDelete(PRBool *_retval)
   input->GetPopupOpen(&isOpen);
   if (!isOpen || mRowCount <= 0) {
     // Nothing left to delete, proceed as normal
-    HandleText(PR_FALSE);
+    HandleText();
     return NS_OK;
   }
 
@@ -1019,7 +1002,8 @@ nsAutoCompleteController::StartSearch()
       PRUint16 searchResult;
       result->GetSearchResult(&searchResult);
       if (searchResult != nsIAutoCompleteResult::RESULT_SUCCESS &&
-          searchResult != nsIAutoCompleteResult::RESULT_SUCCESS_ONGOING)
+          searchResult != nsIAutoCompleteResult::RESULT_SUCCESS_ONGOING &&
+          searchResult != nsIAutoCompleteResult::RESULT_NOMATCH)
         result = nsnull;
     }
 

toolkit/components/passwordmgr/src/nsLoginManager.js

@@ -595,9 +595,11 @@ LoginManager.prototype = {
 
         var result = null;
 
-        if (aPreviousResult) {
+        if (aPreviousResult &&
+                aSearchString.substr(0, aPreviousResult.searchString.length) == aPreviousResult.searchString) {
             this.log("Using previous autocomplete result");
             result = aPreviousResult;
+            result.wrappedJSObject.searchString = aSearchString;
 
             // We have a list of results for a shorter search string, so just
             // filter them further based on the new search string.
@@ -1319,6 +1321,12 @@ UserAutoCompleteResult.prototype = {
     // private
     logins : null,
 
+    // Allow autoCompleteSearch to get at the JS object so it can
+    // modify some readonly properties for internal use.
+    get wrappedJSObject() {
+        return this;
+    },
+
     // Interfaces from idl...
     searchString : null,
     searchResult : Ci.nsIAutoCompleteResult.RESULT_NOMATCH,

toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html

@@ -67,6 +67,13 @@ Login Manager test: multiple login autocomplete
     <input  type="password"   name="pword">
     <button type="submit">Submit</button>
   </form>
+
+  <!-- test autocomplete dropdown -->
+  <form id="form9" action="http://autocomplete5" onsubmit="return false;">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
 </div>
 
 <pre id="test">
@@ -128,6 +135,19 @@ var login6B = new nsLoginInfo(
 var login7  = new nsLoginInfo(
     "http://localhost:8888", "http://autocomplete4", null,
     "form8user", "form8pass", "uname", "pword");
+
+var login8A = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete5", null,
+    "form9userAB", "form9pass", "uname", "pword");
+
+var login8B = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete5", null,
+    "form9userAAB", "form9pass", "uname", "pword");
+
+// login8C is added later
+var login8C = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete5", null,
+    "form9userAABz", "form9pass", "uname", "pword");
 // try/catch in case someone runs the tests manually, twice.
 try {
     pwmgr.addLogin(login0);
@@ -139,6 +159,8 @@ try {
     pwmgr.addLogin(login6A);
     pwmgr.addLogin(login6B);
     pwmgr.addLogin(login7);
+    pwmgr.addLogin(login8A);
+    pwmgr.addLogin(login8B);
 } catch (e) {
     ok(false, "addLogin threw: " + e);
 }
@@ -613,6 +635,41 @@ function runTest(testNum) {
         checkACForm("", "");
         pwmgr.removeLogin(login7);
 
+        testNum = 699;
+        break;
+
+    case 700:
+        // Turn our attention to form9 to test the dropdown - bug 497541
+        uname = $_(9, "uname");
+        pword = $_(9, "pword");
+        sendString("form9userAB", uname);
+        break;
+
+    case 701:
+        checkACForm("form9userAB", "");
+        uname.focus();
+        doKey("left");
+        sendChar("A", uname);
+        break;
+
+    case 702:
+        // check dropdown is updated after inserting "A"
+        checkACForm("form9userAAB", "");
+        checkMenuEntries(["form9userAAB"]);
+        doKey("down");
+        doKey("return");
+        checkACForm("form9userAAB", "form9pass");
+        break;
+
+    case 703:
+        pwmgr.addLogin(login8C);
+        sendChar("z", uname);
+        break;
+
+    case 704:
+        // check that empty results are cached - bug 496466
+        checkMenuEntries([]);
+
         SimpleTest.finish();
         return;
 
@@ -626,7 +683,42 @@ function runTest(testNum) {
 }
 
 
+function checkMenuEntries(expectedValues) {
+    var actualValues = getMenuEntries();
+    is(actualValues.length, expectedValues.length, "Checking length of expected menu");
+    for (var i = 0; i < expectedValues.length; i++)
+        is(actualValues[i], expectedValues[i], "Checking menu entry #"+i);
+}
+
+var autocompletePopup;
+function getMenuEntries() {
+    var entries = [];
+
+    // Could perhaps pull values directly from the controller, but it seems
+    // more reliable to test the values that are actually in the tree?
+    var column = autocompletePopup.tree.columns[0];
+    var numRows = autocompletePopup.tree.view.rowCount;
+    for (var i = 0; i < numRows; i++) {
+        entries.push(autocompletePopup.tree.view.getValueAt(i, column));
+    }
+    return entries;
+}
+
 function startTest() {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    var Ci = Components.interfaces;
+    chromeWin = window
+                    .QueryInterface(Ci.nsIInterfaceRequestor)
+                    .getInterface(Ci.nsIWebNavigation)
+                    .QueryInterface(Ci.nsIDocShellTreeItem)
+                    .rootTreeItem
+                    .QueryInterface(Ci.nsIInterfaceRequestor)
+                    .getInterface(Ci.nsIDOMWindow)
+                    .QueryInterface(Ci.nsIDOMChromeWindow);
+    // shouldn't reach into browser internals like this and
+    // shouldn't assume ID is consistent across products
+    autocompletePopup = chromeWin.document.getElementById("PopupAutoComplete");
+    ok(autocompletePopup, "Got autocomplete popup");
     runTest(1);
 }
 

toolkit/components/satchel/src/nsFormAutoComplete.js

@@ -152,9 +152,11 @@ FormAutoComplete.prototype = {
 
         let result = null;
 
-        if (aPreviousResult) {
+        if (aPreviousResult &&
+                aSearchString.substr(0, aPreviousResult.searchString.length) == aPreviousResult.searchString) {
             this.log("Using previous autocomplete result");
             result = aPreviousResult;
+            result.wrappedJSObject.searchString = aSearchString;
 
             // We have a list of results for a shorter search string, so just
             // filter them further based on the new search string.
@@ -263,6 +265,12 @@ FormAutoCompleteResult.prototype = {
             throw Components.Exception("Index out of range.", Cr.NS_ERROR_ILLEGAL_VALUE);
     },
 
+    // Allow autoCompleteSearch to get at the JS object so it can
+    // modify some readonly properties for internal use.
+    get wrappedJSObject() {
+        return this;
+    },
+
     // Interfaces from idl...
     searchString : null,
     searchResult : Ci.nsIAutoCompleteResult.RESULT_NOMATCH,

toolkit/components/satchel/src/nsFormFillController.cpp

@@ -667,7 +667,7 @@ nsFormFillController::KeyPress(nsIDOMEvent* aEvent)
     mController->HandleDelete(&cancel);
     break;
   case nsIDOMKeyEvent::DOM_VK_BACK_SPACE:
-    mController->HandleText(PR_FALSE);
+    mController->HandleText();
     break;
 #else
   case nsIDOMKeyEvent::DOM_VK_BACK_SPACE:
@@ -678,7 +678,7 @@ nsFormFillController::KeyPress(nsIDOMEvent* aEvent)
       if (isShift)
         mController->HandleDelete(&cancel);
       else
-        mController->HandleText(PR_FALSE);
+        mController->HandleText();
 
       break;
     }
@@ -787,7 +787,7 @@ nsFormFillController::Input(nsIDOMEvent* aEvent)
   if (mSuppressOnInput || !mController || !mFocusedInput)
     return NS_OK;
 
-  return mController->HandleText(PR_FALSE);
+  return mController->HandleText();
 }
 
 ////////////////////////////////////////////////////////////////////////
@@ -829,7 +829,7 @@ nsFormFillController::MouseDown(nsIDOMEvent* aMouseEvent)
   if (value.Length() > 0) {
     // Show the popup with a filtered result set
     mController->SetSearchString(EmptyString());
-    mController->HandleText(PR_TRUE);
+    mController->HandleText();
   } 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.

toolkit/components/satchel/test/test_form_autocomplete.html

@@ -70,6 +70,7 @@ fh.addEntry("field1", "value4");
 fh.addEntry("field2", "value1");
 fh.addEntry("field3", "a");
 fh.addEntry("field3", "aa");
+fh.addEntry("field3", "aaz");
 fh.addEntry("field3", "aa\xe6"); // 0xae == latin ae pair (0xc6 == AE)
 fh.addEntry("field3", "az");
 fh.addEntry("field3", "z");
@@ -410,17 +411,53 @@ function runTest(testNum) {
     /* Test filtering as characters are typed. */
 
     case 200:
-        checkMenuEntries(["a", "aa", "aa\xe6", "az"]);
+        checkMenuEntries(["a", "aa", "aaz", "aa\xe6", "az"]);
         sendChar("a", input);
         break;
 
     case 201:
-        checkMenuEntries(["aa", "aa\xe6"]);
+        checkMenuEntries(["aa", "aaz", "aa\xe6"]);
         sendChar("\xc6", input);
         break;
 
     case 202:
         checkMenuEntries(["aa\xe6"]);
+        doKey("back_space");
+        break;
+
+    case 203:
+        checkMenuEntries(["aa", "aaz", "aa\xe6"]);
+        doKey("back_space");
+        break;
+
+    case 204:
+        checkMenuEntries(["a", "aa", "aaz", "aa\xe6", "az"]);
+        sendChar("z", input);
+        break;
+
+    case 205:
+        checkMenuEntries(["az"]);
+        doKey("left");
+        sendChar("a", input);
+        break;
+
+    case 206:
+        checkMenuEntries(["aaz"]);
+        fh.addEntry("field3", "aazq");
+        doKey("right");
+        sendChar("q", input);
+        break;
+
+    case 207:
+        // check that results were cached
+        checkMenuEntries([]);
+        fh.addEntry("field3", "aazqq");
+        sendChar("q", input);
+        break;
+
+    case 208:
+        // check that empty results were cached - bug 496466
+        checkMenuEntries([]);
         doKey("escape");
 
         SimpleTest.finish();