Bug 400795 - initial form fill and username autocomplete should share common code. r=gavin

toolkit/components/passwordmgr/src/nsLoginManager.js

@@ -325,17 +325,24 @@ LoginManager.prototype = {
         handleEvent : function (event) {
             this._pwmgr.log("domEventListener: got event " + event.type);
 
-            var doc, inputElement;
             switch (event.type) {
                 case "DOMContentLoaded":
-                    doc = event.target;
+                    this._pwmgr._fillDocument(event.target);
-                    this._pwmgr._fillDocument(doc);
                     return;
 
                 case "DOMAutoComplete":
                 case "blur":
-                    inputElement = event.target;
+                    var acInputField = event.target;
-                    this._pwmgr._fillPassword(inputElement);
+                    var acForm = acInputField.form;
+                    // Make sure the username field fillForm will use is the
+                    // same field as the autocomplete was activated on. If
+                    // not, the DOM has been altered and we'll just give up.
+                    var [usernameField, passwordField, ignored] =
+                        this._pwmgr._getFormFields(acForm, false);
+                    if (usernameField == acInputField)
+                        this._pwmgr._fillForm(acForm, true, true, null);
+                    else
+                        this._pwmgr.log("Oops, form changed before AC invoked");
                     return;
 
                 default:
@@ -1159,63 +1166,6 @@ LoginManager.prototype = {
         element.addEventListener("DOMAutoComplete",
                                 this._domEventListener, false);
         this._formFillService.markAsLoginManagerField(element);
-    },
-
-
-    /*
-     * _fillPassword
-     *
-     * The user has autocompleted a username field, so fill in the password.
-     */
-    _fillPassword : function (usernameField) {
-        this.log("fillPassword autocomplete username: " + usernameField.value);
-
-        var form = usernameField.form;
-        var doc = form.ownerDocument;
-
-        var hostname = this._getPasswordOrigin(doc.documentURI);
-        var formSubmitURL = this._getActionOrigin(form)
-
-        // Find the password field. We should always have at least one,
-        // or else something has gone rather wrong.
-        var pwFields = this._getPasswordFields(form, false);
-        if (!pwFields) {
-            const err = "No password field for autocomplete password fill.";
-
-            // We want to know about this even if debugging is disabled.
-            if (!this._debug)
-                dump(err);
-            else
-                this.log(err);
-
-            return;
-        }
-
-        // If there are multiple passwords fields, we can't really figure
-        // out what each field is for, so just fill out the last field.
-        var passwordField = pwFields[0].element;
-
-        // Temporary LoginInfo with the info we know.
-        var currentLogin = new this._nsLoginInfo();
-        currentLogin.init(hostname, formSubmitURL, null,
-                          usernameField.value, null,
-                          usernameField.name, passwordField.name);
-
-        // Look for a existing login and use its password.
-        var match = null;
-        var logins = this.findLogins({}, hostname, formSubmitURL, null);
-
-        if (!logins.some(function(l) {
-                                match = l;
-                                return currentLogin.matches(l, true);
-                        }))
-        {
-            this.log("Can't find a login for this autocomplete result.");
-            return;
-        }
-
-        this.log("Found a matching login, filling in password.");
-        passwordField.value = match.password;
     }
 }; // end of LoginManager implementation
 
@@ -1295,7 +1245,7 @@ UserAutoCompleteResult.prototype = {
                         getService(Ci.nsILoginManager);
             pwmgr.removeLogin(removedLogin);
         }
-    },
+    }
 };
 
 var component = [LoginManager];

toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html

@@ -53,6 +53,13 @@ Login Manager test: multiple login autocomplete
     <input  type="password"   name="pword">
     <button type="submit">Submit</button>
   </form>
+
+  <!-- This form will be manipulated to insert a different username field. -->
+  <form id="form7" action="http://autocomplete3" onsubmit="return false;">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
 </div>
 
 <pre id="test">
@@ -103,6 +110,13 @@ var login5 = new nsLoginInfo(
     "http://localhost:8888", "http://autocomplete2", null,
     "singleuser5", "singlepass5", "uname", "pword");
 
+var login6A = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete3", null,
+    "form7user1", "form7pass1", "uname", "pword");
+var login6B = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete3", null,
+    "form7user2", "form7pass2", "uname", "pword");
+
 // try/catch in case someone runs the tests manually, twice.
 try {
     pwmgr.addLogin(login1);
@@ -110,6 +124,8 @@ try {
     pwmgr.addLogin(login3);
     pwmgr.addLogin(login4);
     pwmgr.addLogin(login5);
+    pwmgr.addLogin(login6A);
+    pwmgr.addLogin(login6B);
 } catch (e) {
     ok(false, "addLogin threw: " + e);
 }
@@ -534,10 +550,53 @@ function runTest(testNum) {
         pword = $_(6, "pword");
         checkACForm("singleuser5", "singlepass5");
 
-        /* FALLTHRU */
+        pwmgr.removeLogin(login5);
+        testNum = 499;
+        break;
+
+    case 500:
+        // Turn our attention to form7
+        uname = $_(7, "uname");
+        pword = $_(7, "pword");
+        checkACForm("", "");
+
+        // Insert a new username field into the form. We'll then make sure
+        // that invoking the autocomplete doesn't try to fill the form.
+        var newField = document.createElement("input");
+        newField.setAttribute("type", "text");
+        newField.setAttribute("name", "uname2");
+        pword.parentNode.insertBefore(newField, pword);
+        is($_(7, "uname2").value, "", "Verifying empty uname2");;
+
+        // Delete login6B. It was created just to prevent filling in a login
+        // automatically, removing it makes it more likely that we'll catch a
+        // future regression with form filling here.
+        pwmgr.removeLogin(login6B);
+
+        // Trigger autocomplete popup
+        restoreForm();
+        doKey("down");
+        break;
+
+    case 501:
+        // Check first entry
+        doKey("down");
+        checkACForm("", ""); // value shouldn't update
+        doKey("return"); // not "enter"!
+        // The form changes, so we expect the old username field to get the
+        // selected autocomplete value, but neither the new username field nor
+        // the password field should have any values filled in.
+        checkACForm("form7user1", "");
+        is($_(7, "uname2").value, "", "Verifying empty uname2");;
+
+        pwmgr.removeLogin(login6A);
+
+
+        SimpleTest.finish();
+        return;
 
     default:
-        pwmgr.removeLogin(login5);
+        ok(false, "Unexpected invocataion of test #" + testNum);
         SimpleTest.finish();
         return;
   }