Bug 446109 - Password manager can overwrite an existing password in an input. r=gavin
authorJustin Dolske <dolske@mozilla.com>
Sat, 06 Sep 2008 23:52:15 -0700
changeset 18921 eb0decf65376a5f8fcc5438394850a6129f6a046
parent 18920 443478589683bc530a11d4906c93f8e1e18847ed
child 18922 57feb1188e7d332340328c3d8a50a66d0bda244f
push id1827
push userjdolske@mozilla.com
push dateSun, 07 Sep 2008 06:52:25 +0000
treeherderautoland@eb0decf65376 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin
bugs446109
milestone1.9.1b1pre
Bug 446109 - Password manager can overwrite an existing password in an input. r=gavin
toolkit/components/passwordmgr/src/nsLoginManager.js
toolkit/components/passwordmgr/test/test_basic_form_1pw.html
toolkit/components/passwordmgr/test/test_basic_form_2pw_1.html
--- a/toolkit/components/passwordmgr/src/nsLoginManager.js
+++ b/toolkit/components/passwordmgr/src/nsLoginManager.js
@@ -334,20 +334,23 @@ LoginManager.prototype = {
                 case "blur":
                     var acInputField = event.target;
                     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)
+                    if (usernameField == acInputField && passwordField) {
+                        // Clobber any existing password.
+                        passwordField.value = "";
                         this._pwmgr._fillForm(acForm, true, true, null);
-                    else
+                    } else {
                         this._pwmgr.log("Oops, form changed before AC invoked");
+                    }
                     return;
 
                 default:
                     this._pwmgr.log("Oops! This event unexpected.");
                     return;
             }
         }
     },
@@ -1012,21 +1015,16 @@ LoginManager.prototype = {
         // If the fields are disabled or read-only, there's nothing to do.
         if (passwordField.disabled || passwordField.readOnly ||
             usernameField && (usernameField.disabled ||
                               usernameField.readOnly)) {
             this.log("not filling form, login fields disabled");
             return [false, foundLogins];
         }
 
-        // If there's only a password field and it has a value, there's
-        // nothing for us to do. (Don't clobber the existing value)
-        if (!usernameField && passwordField.value)
-            return [false, foundLogins];
-
         // Need to get a list of logins if we weren't given them
         if (foundLogins == null) {
             var formOrigin = 
                 this._getPasswordOrigin(form.ownerDocument.documentURI);
             var actionOrigin = this._getActionOrigin(form);
             foundLogins = this.findLogins({}, formOrigin, actionOrigin, null);
             this.log("found " + foundLogins.length + " matching logins.");
         } else {
@@ -1062,16 +1060,20 @@ LoginManager.prototype = {
 
 
         // Attach autocomplete stuff to the username field, if we have
         // one. This is normally used to select from multiple accounts,
         // but even with one account we should refill if the user edits.
         if (usernameField)
             this._attachToInput(usernameField);
 
+        // Don't clobber an existing password.
+        if (passwordField.value)
+            return [false, foundLogins];
+
         // If the form has an autocomplete=off attribute in play, don't
         // fill in the login automatically. We check this after attaching
         // the autocomplete stuff to the username field, so the user can
         // still manually select a login to be filled in.
         var isFormDisabled = false;
         if (!ignoreAutocomplete &&
             (this._isAutocompleteDisabled(form) ||
              this._isAutocompleteDisabled(usernameField) ||
--- a/toolkit/components/passwordmgr/test/test_basic_form_1pw.html
+++ b/toolkit/components/passwordmgr/test/test_basic_form_1pw.html
@@ -2737,52 +2737,54 @@ if ($noUsername) { next STRUCTURE; }
 
 /** Test for Login Manager: simple form fill **/
 
 function startTest() {
     var f;
 
     for (f = 1;   f <= 4;   f++) { checkForm(f, "testpass"); }
     for (f = 5;   f <= 6;   f++) { checkUnmodifiedForm(f); }
-    for (f = 7;   f <= 14;   f++) { checkForm(f, "testuser", "testpass"); }
-    todo(false, "Don't clobber prefilled differing password");
-    //for (f = 15;   f <= 18;  f++) { checkUnmodifiedForm(f); }
+    for (f = 7;   f <= 10;  f++) { checkForm(f, "testuser", "testpass"); }
+    // 11-14 have the password prefilled, we skip filling the username.
+    for (f = 11;  f <= 14;  f++) { checkUnmodifiedForm(f); }
+    for (f = 15;  f <= 18;  f++) { checkUnmodifiedForm(f); }
     for (f = 19;  f <= 26;  f++) { checkForm(f, "testuser", "testpass"); }
-    todo(false, "Don't clobber prefilled differing password");
-    //for (f = 27;  f <= 30;  f++) { checkUnmodifiedForm(f); }
+    for (f = 27;  f <= 30;  f++) { checkUnmodifiedForm(f); }
     for (f = 31;  f <= 42;  f++) { checkUnmodifiedForm(f); }
 
     todo(false, "Add support for password before username");
     //for (f = 43;  f <= 50;  f++) { checkForm(f, "testpass", "testuser"); }
     //for (f = 51;  f <= 54;  f++) { checkUnmodifiedForm(f); }
     //for (f = 55;  f <= 62;  f++) { checkForm(f, "testpass", "testuser"); }
     //for (f = 63;  f <= 78;  f++) { checkUnmodifiedForm(f); }
 
     //for (f = 79;  f <= 90;  f++) { checkUnmodifiedForm(f); }
 
-    for (f = 91;  f <= 98;  f++) { checkForm(f, "testuser", "testpass", ""); }
-    todo(false, "Don't clobber prefilled differing password");
-    //for (f = 99;  f <= 102; f++) { checkUnmodifiedForm(f); }
+    for (f = 91;  f <= 94;  f++) { checkForm(f, "testuser", "testpass", ""); }
+    // 95-98 have the password prefilled, we skip filling the username.
+    for (f = 95;  f <= 98;  f++) { checkUnmodifiedForm(f); }
+    for (f = 99;  f <= 102; f++) { checkUnmodifiedForm(f); }
     for (f = 103; f <= 110; f++) { checkForm(f, "testuser", "testpass", ""); }
-    todo(false, "Don't clobber prefilled differing password");
-    //for (f = 111;  f <= 114;  f++) { checkUnmodifiedForm(f); }
-    for (f = 115;  f <= 126; f++) { checkUnmodifiedForm(f); }
+    for (f = 111; f <= 114; f++) { checkUnmodifiedForm(f); }
+    for (f = 115; f <= 126; f++) { checkUnmodifiedForm(f); }
 
     todo(false, "Ambigious username fields,");
     //for (f = 127; f <= 134; f++) { checkForm(f, "testuser", "", "testpass"); }
 
     todo(false, "More of the same, just hard to split up...");
     //for (f = 135; f <= 138; f++) { checkUnmodifiedForm(f); }
     //for (f = 139; f <= 146; f++) { checkForm(f, "testuser", "", "testpass"); }
     //for (f = 147; f <= 162; f++) { checkUnmodifiedForm(f); }
 
-    for (f = 163; f <= 170; f++) { checkForm(f, "", "testuser", "testpass"); }
-    //for (f = 171; f <= 174; f++) { checkUnmodifiedForm(f); }
+    for (f = 163; f <= 166; f++) { checkForm(f, "", "testuser", "testpass"); }
+    // 167-170 have the password prefilled, we skip filling the username.
+    for (f = 167; f <= 170; f++) { checkUnmodifiedForm(f); }
+    for (f = 171; f <= 174; f++) { checkUnmodifiedForm(f); }
     for (f = 175; f <= 182; f++) { checkForm(f, "", "testuser", "testpass"); }
-    //for (f = 183; f <= 186; f++) { checkUnmodifiedForm(f); }
+    for (f = 183; f <= 186; f++) { checkUnmodifiedForm(f); }
     for (f = 187; f <= 198; f++) { checkUnmodifiedForm(f); }
 
     todo(false, "Add support for password before username");
     //for (f = 199; f <= 206; f++) { checkForm(f, "testpass", "testuser", ""); }
     //for (f = 207; f <= 210; f++) { checkUnmodifiedForm(f); }
     //for (f = 211; f <= 218; f++) { checkForm(f, "testpass", "testuser", ""); }
     //for (f = 219; f <= 234; f++) { checkUnmodifiedForm(f); }
 
--- a/toolkit/components/passwordmgr/test/test_basic_form_2pw_1.html
+++ b/toolkit/components/passwordmgr/test/test_basic_form_2pw_1.html
@@ -15629,20 +15629,21 @@ print "</form>\n\n";
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 /** Test for Login Manager: simple form fill **/
 
 function startTest() {
     var f;
 
-    for (f = 1;   f <= 64;  f++) { checkForm(f, "testuser", "testpass", null); }
-
-    todo(false, "testing forms 65-96 disabled");
-    //for (f = 65;  f <= 96;  f++) { checkUnmodifiedForm(f); }
+    for (f = 1;   f <= 32;  f++) { checkForm(f, "testuser", "testpass", null); }
+    // 33-64 have the password prefilled, we skip filling the username.
+    for (f = 33;  f <= 64;  f++) { checkUnmodifiedForm(f); }
+
+    for (f = 65;  f <= 96;  f++) { checkUnmodifiedForm(f); }
     todo(false, "testing forms 101-192 disabled");
     //for (f = 97;  f <= 160; f++) { checkForm(f, "testuser", "testpass", null); }
     //for (f = 161; f <= 192; f++) { checkUnmodifiedForm(f); }
     for (f = 193; f <= 288; f++) { checkUnmodifiedForm(f); }
 
     todo(false, "testing forms 289-480 disabled");
     //for (f = 289; f <= 352; f++) { checkForm(f, "testuser", null, "testpass"); }
     //for (f = 353; f <= 384; f++) { checkUnmodifiedForm(f); }