From 64cba7657782813822efe5e9b3ce1aeaab1a570a Mon Sep 17 00:00:00 2001 From: "dolske@mozilla.com" Date: Thu, 8 May 2008 20:34:46 -0700 Subject: [PATCH] Bug 362576 - autocomplete="off" should prevent filling passwords in addition to remembering passwords. r=gavin, ui-r=mconnor, a1.9=mconnor --- .../passwordmgr/src/nsLoginManager.js | 46 ++++-- .../test/test_basic_form_autocomplete.html | 137 +++++++++++++++++- .../passwordmgr/test/test_bug_227640.html | 9 +- .../satchel/src/nsFormFillController.cpp | 10 +- 4 files changed, 179 insertions(+), 23 deletions(-) diff --git a/toolkit/components/passwordmgr/src/nsLoginManager.js b/toolkit/components/passwordmgr/src/nsLoginManager.js index 43cfd819b45..9d82f85405c 100644 --- a/toolkit/components/passwordmgr/src/nsLoginManager.js +++ b/toolkit/components/passwordmgr/src/nsLoginManager.js @@ -715,6 +715,20 @@ LoginManager.prototype = { }, + /* + * _isAutoCompleteDisabled + * + * Returns true if the page requests autocomplete be disabled for the + * specified form input. + */ + _isAutocompleteDisabled : function (element) { + if (element && element.hasAttribute("autocomplete") && + element.getAttribute("autocomplete").toLowerCase() == "off") + return true; + + return false; + }, + /* * _onFormSubmit * @@ -725,15 +739,6 @@ LoginManager.prototype = { */ _onFormSubmit : function (form) { - // local helper function - function autocompleteDisabled(element) { - if (element && element.hasAttribute("autocomplete") && - element.getAttribute("autocomplete").toLowerCase() == "off") - return true; - - return false; - }; - // local helper function function getPrompter(aWindow) { var prompterSvc = Cc["@mozilla.org/login-manager/prompter;1"]. @@ -769,10 +774,10 @@ LoginManager.prototype = { // Check for autocomplete=off attribute. We don't use it to prevent // autofilling (for existing logins), but won't save logins when it's // present. - if (autocompleteDisabled(form) || - autocompleteDisabled(usernameField) || - autocompleteDisabled(newPasswordField) || - autocompleteDisabled(oldPasswordField)) { + if (this._isAutocompleteDisabled(form) || + this._isAutocompleteDisabled(usernameField) || + this._isAutocompleteDisabled(newPasswordField) || + this._isAutocompleteDisabled(oldPasswordField)) { this.log("(form submission ignored -- autocomplete=off found)"); return; } @@ -1014,7 +1019,20 @@ LoginManager.prototype = { if (usernameField) this._attachToInput(usernameField); - if (autofillForm) { + // 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 (this._isAutocompleteDisabled(form) || + this._isAutocompleteDisabled(usernameField) || + this._isAutocompleteDisabled(passwordField)) { + + isFormDisabled = true; + this.log("form[" + i + "]: not filled, has autocomplete=off"); + } + + if (autofillForm && !isFormDisabled) { if (usernameField && usernameField.value) { // If username was specified in the form, only fill in the diff --git a/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html b/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html index 2fe365aae8e..61202bd9419 100644 --- a/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html +++ b/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html @@ -15,14 +15,44 @@ Login Manager test: multiple login autocomplete
+
- -
+ +
+ + + +
+ +
+ + + +
+ +
+ + + +
+ +
+ + + +
+ + +
+ + + + 
@@ -68,12 +98,18 @@ var login4 = new nsLoginInfo(
     "http://localhost:8888", "http://autocomplete:8888", null,
     "zzzuser4", "zzzpass4", "uname", "pword");
 
+// login 5 only used in the single-user forms
+var login5 = new nsLoginInfo(
+    "http://localhost:8888", "http://autocomplete2", null,
+    "singleuser5", "singlepass5", "uname", "pword");
+
 // try/catch in case someone runs the tests manually, twice.
 try {
     pwmgr.addLogin(login1);
     pwmgr.addLogin(login2);
     pwmgr.addLogin(login3);
     pwmgr.addLogin(login4);
+    pwmgr.addLogin(login5);
 } catch (e) {
     ok(false, "addLogin threw: " + e);
 }
@@ -89,8 +125,9 @@ function restoreForm() {
 
 // Check for expected username/password in form.
 function checkACForm(expectedUsername, expectedPassword) {
-  is(uname.value, expectedUsername, "Checking form username");
-  is(pword.value, expectedPassword, "Checking form password");
+  var formID = uname.parentNode.id;
+  is(uname.value, expectedUsername, "Checking " + formID + " username");
+  is(pword.value, expectedPassword, "Checking " + formID + " password");
 }
 
 
@@ -407,8 +444,100 @@ function runTest(testNum) {
         checkACForm("", "");
         numLogins = pwmgr.countLogins("http://localhost:8888", "http://autocomplete:8888", null);
         is(numLogins, 0, "Correct number of logins after deleting one");
+        testNum = 99;
+        break;
+
+
+    /* Tests for single-user forms with autocomplete=off */
+
+    case 100:
+        // Turn our attention to form2
+        uname = $_(2, "uname");
+        pword = $_(2, "pword");
+        checkACForm("", "");
+
+        // Trigger autocomplete popup
+        restoreForm();
+        doKey("down");
+        break;
+
+    case 101:
+        // Check first entry
+        doKey("down");
+        checkACForm("", ""); // value shouldn't update
+        doKey("return"); // not "enter"!
+        checkACForm("singleuser5", "singlepass5");
+        break;
+
+    case 102:
+        // Turn our attention to form3
+        uname = $_(3, "uname");
+        pword = $_(3, "pword");
+        checkACForm("", "");
+
+        // Trigger autocomplete popup
+        restoreForm();
+        doKey("down");
+        break;
+
+    case 103:
+        // Check first entry
+        doKey("down");
+        checkACForm("", ""); // value shouldn't update
+        doKey("return"); // not "enter"!
+        checkACForm("singleuser5", "singlepass5");
+        break;
+
+    case 104:
+        // Turn our attention to form4
+        uname = $_(4, "uname");
+        pword = $_(4, "pword");
+        checkACForm("", "");
+
+        // Trigger autocomplete popup
+        restoreForm();
+        doKey("down");
+        break;
+
+    case 105:
+        // Check first entry
+        doKey("down");
+        checkACForm("", ""); // value shouldn't update
+        doKey("return"); // not "enter"!
+        checkACForm("singleuser5", "singlepass5");
+        break;
+
+    case 106:
+        // Turn our attention to form5
+        uname = $_(5, "uname");
+        pword = $_(5, "pword");
+        checkACForm("", "");
+
+        // Trigger autocomplete popup
+        restoreForm();
+        doKey("down");
+        break;
+
+    case 107:
+        // Check first entry
+        doKey("down");
+        checkACForm("", ""); // value shouldn't update
+        doKey("return"); // not "enter"!
+        checkACForm("singleuser5", "singlepass5");
+        break;
+
+    case 108:
+        // Turn our attention to form6
+        // (this is a control, w/o autocomplete=off, to ensure the login
+        // that was being suppressed would have been filled in otherwise)
+        uname = $_(6, "uname");
+        pword = $_(6, "pword");
+        checkACForm("singleuser5", "singlepass5");
+
+        /* FALLTHRU */
 
     default:
+        pwmgr.removeLogin(login5);
         SimpleTest.finish();
         return;
   }
diff --git a/toolkit/components/passwordmgr/test/test_bug_227640.html b/toolkit/components/passwordmgr/test/test_bug_227640.html
index 07afdced49d..f67e33aaaf8 100644
--- a/toolkit/components/passwordmgr/test/test_bug_227640.html
+++ b/toolkit/components/passwordmgr/test/test_bug_227640.html
@@ -161,10 +161,13 @@ function startTest() {
   numStartingLogins = countLogins();
   ok(numStartingLogins > 0, "counting logins at start");
 
-  // Check first set of forms, which should be filled by pwmgr.
+  // Check first set of forms, which should not be filled by pwmgr.
   for (var i = 1; i <= 7; i++) {
-    is($_(i, "uname").value, "testuser", "Checking for filled username " + i);
-    is($_(i, "pword").value, "testpass", "Checking for filled password " + i);
+    is($_(i, "uname").value, "", "Checking for unfilled username " + i);
+    is($_(i, "pword").value, "", "Checking for unfilled password " + i);
+    // Set the field values to that of an existing login
+    $_(i, "uname").value = "testuser";
+    $_(i, "pword").value = "testpass";
   }
 
   // Check second set of forms, which should have preset values (and are unknown to pwmgr).
diff --git a/toolkit/components/satchel/src/nsFormFillController.cpp b/toolkit/components/satchel/src/nsFormFillController.cpp
index 4f0e8eb8caa..642674f0d31 100644
--- a/toolkit/components/satchel/src/nsFormFillController.cpp
+++ b/toolkit/components/satchel/src/nsFormFillController.cpp
@@ -608,15 +608,21 @@ nsFormFillController::Focus(nsIDOMEvent* aEvent)
                                   
     nsAutoString autocomplete; 
     input->GetAttribute(NS_LITERAL_STRING("autocomplete"), autocomplete);
+
+    PRInt32 dummy;
+    PRBool isPwmgrInput = PR_FALSE;
+    if (mPwmgrInputs.Get(input, &dummy))
+        isPwmgrInput = PR_TRUE;
+
     if (type.LowerCaseEqualsLiteral("text") && !isReadOnly &&
-        !autocomplete.LowerCaseEqualsLiteral("off")) {
+        (!autocomplete.LowerCaseEqualsLiteral("off") || isPwmgrInput)) {
 
       nsCOMPtr form;
       input->GetForm(getter_AddRefs(form));
       if (form)
         form->GetAttribute(NS_LITERAL_STRING("autocomplete"), autocomplete);
 
-      if (!form || !autocomplete.LowerCaseEqualsLiteral("off"))
+      if (!form || !autocomplete.LowerCaseEqualsLiteral("off") || isPwmgrInput)
         StartControllingInput(input);
     }