Bug 1155390 - Don't prompt to update a password when there is no username field and the password is identical. r=dolske
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 28 Apr 2015 11:30:47 -0700
changeset 241417 e93eb477c5aa05405f43bac72566177440da2594
parent 241416 3f2af35fb884b1b50ca9d6d2a418a9f3e194f00e
child 241472 ff7b5b03fece6c5d2e3fee12cfe6311ad45be934
push id12628
push usermozilla@noorenberghe.ca
push dateTue, 28 Apr 2015 18:31:34 +0000
treeherderfx-team@e93eb477c5aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske
bugs1155390
milestone40.0a1
Bug 1155390 - Don't prompt to update a password when there is no username field and the password is identical. r=dolske subtst_notifications_2pw_0un.html was updated so it could be re-used for case 27 and 28 with the existing test logins there.
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/test/subtst_notifications_2pw_0un.html
toolkit/components/passwordmgr/test/test_notifications.html
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -1,9 +1,8 @@
-/* vim: set ts=2 sts=2 sw=2 et tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
@@ -403,16 +402,25 @@ var LoginManagerParent = {
       prompterSvc.init(target.isRemoteBrowser ?
                           target.ownerDocument.defaultView :
                           target.contentWindow);
       if (target.isRemoteBrowser)
         prompterSvc.setE10sData(target, opener);
       return prompterSvc;
     }
 
+    function recordLoginUse(login) {
+      // Update the lastUsed timestamp and increment the use count.
+      let propBag = Cc["@mozilla.org/hash-property-bag;1"].
+                    createInstance(Ci.nsIWritablePropertyBag);
+      propBag.setProperty("timeLastUsed", Date.now());
+      propBag.setProperty("timesUsedIncrement", 1);
+      Services.logins.modifyLogin(login, propBag);
+    }
+
     if (!Services.logins.getLoginSavingEnabled(hostname)) {
       log("(form submission ignored -- saving is disabled for:", hostname, ")");
       return;
     }
 
     var formLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
                     createInstance(Ci.nsILoginInfo);
     formLogin.init(hostname, formSubmitURL, null,
@@ -426,16 +434,24 @@ var LoginManagerParent = {
     // If we didn't find a username field, but seem to be changing a
     // password, allow the user to select from a list of applicable
     // logins to update the password for.
     if (!usernameField && oldPasswordField && logins.length > 0) {
       var prompter = getPrompter();
 
       if (logins.length == 1) {
         var oldLogin = logins[0];
+
+        if (oldLogin.password == formLogin.password) {
+          recordLoginUse(oldLogin);
+          log("(Not prompting to save/change since we have no username and the " +
+              "only saved password matches the new password)");
+          return;
+        }
+
         formLogin.username      = oldLogin.username;
         formLogin.usernameField = oldLogin.usernameField;
 
         prompter.promptToChangePassword(oldLogin, formLogin);
       } else {
         prompter.promptToChangePasswordWithUsernames(
                             logins, logins.length, formLogin);
       }
@@ -476,22 +492,17 @@ var LoginManagerParent = {
       log("Found an existing login matching this form submission");
 
       // Change password if needed.
       if (existingLogin.password != formLogin.password) {
         log("...passwords differ, prompting to change.");
         prompter = getPrompter();
         prompter.promptToChangePassword(existingLogin, formLogin);
       } else {
-        // Update the lastUsed timestamp.
-        var propBag = Cc["@mozilla.org/hash-property-bag;1"].
-                      createInstance(Ci.nsIWritablePropertyBag);
-        propBag.setProperty("timeLastUsed", Date.now());
-        propBag.setProperty("timesUsedIncrement", 1);
-        Services.logins.modifyLogin(existingLogin, propBag);
+        recordLoginUse(existingLogin);
       }
 
       return;
     }
 
 
     // Prompt user to save login (via dialog or notification bar)
     prompter = getPrompter();
--- a/toolkit/components/passwordmgr/test/subtst_notifications_2pw_0un.html
+++ b/toolkit/components/passwordmgr/test/subtst_notifications_2pw_0un.html
@@ -7,17 +7,17 @@
 <form id="form" action="formsubmit.sjs">
   <input id="pass1" name="pass1" type="password" value="staticpw">
   <input id="pass" name="pass" type="password">
   <button type="submit">Submit</button>
 </form>
 
 <script>
 function submitForm() {
-  pass.value = "notifyp2";
+  pass.value = "notifyp1";
   form.submit();
 }
 
 window.onload = submitForm;
 var form      = document.getElementById("form");
 var pass      = document.getElementById("pass");
 
 </script>
--- a/toolkit/components/passwordmgr/test/test_notifications.html
+++ b/toolkit/components/passwordmgr/test/test_notifications.html
@@ -46,16 +46,18 @@ var subtests = [
                    "subtst_notifications_9.html", // 19
                    "subtst_notifications_10.html",  // 20
                    "http://test1.example.org:80" + testpath + "subtst_notifications_1.html", // 21
                    "http://test1.example.org:80" + testpath + "subtst_notifications_7.html", // 22
                    "http://test1.example.org:80" + testpath + "subtst_notifications_6.html", // 23
                    "subtst_notifications_2pw_0un.html",  // 24
                    "subtst_notifications_2pw_0un.html",  // 25
                    "subtst_notifications_2pw_0un.html",  // 26
+                   "subtst_notifications_2pw_0un.html",  // 27
+                   "subtst_notifications_2pw_0un.html",  // 28
                ];
 
 
 var ignoreLoad = false;
 function handleLoad(aEvent) {
     // ignore every other load event ... We get one for loading the subtest (which
     // we want to ignore), and another when the subtest's form submits itself
     // (which we want to handle, to start the next test).
@@ -142,17 +144,17 @@ function checkTest() {
       case 5:
         // Same subtest, make sure we didn't prompt for an existing login.
         is(gotUser, "notifyu1", "Checking submitted username");
         is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-save");
         ok(!popup, "checking for no notification popup");
 
         // Check to make sure we updated the timestamps and use count on the
-        // existing loging that was submitted for this form.
+        // existing login that was submitted for this form.
         var logins = pwmgr.getAllLogins();
         is(logins.length, 1, "Should only have 1 login");
         ok(SpecialPowers.call_Instanceof(logins[0], Ci.nsILoginMetaInfo), "metainfo QI");
         is(logins[0].timesUsed, 2, "check .timesUsed for existing login submission");
         ok(logins[0].timeLastUsed > logins[0].timeCreated, "timeLastUsed bumped");
         ok(logins[0].timeCreated == logins[0].timePasswordChanged, "timeChanged not updated");
 
         // remove that login
@@ -380,50 +382,99 @@ function checkTest() {
         ok(expectedText.test(notificationText), "Checking text: " + notificationText);
         popup.remove();
         break;
 
       case 24:
         // Check for notification popup when a form with 2 password fields (no username) is
         // submitted and there are no saved logins.
         is(gotUser, "null", "Checking submitted username");
-        is(gotPass, "notifyp2", "Checking submitted password");
+        is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-save");
         ok(popup, "got notification popup");
         popup.remove();
 
         // Add login for the next test
-        pwmgr.addLogin(login1);
+        pwmgr.addLogin(login1B);
         break;
 
       case 25:
         // Check for notification popup when a form with 2 password fields (no username) is
         // submitted and there is a saved login with a username and different password.
         is(gotUser, "null", "Checking submitted username");
-        is(gotPass, "notifyp2", "Checking submitted password");
+        is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-change");
         ok(popup, "got notification popup");
         popup.remove();
         // remove that login
-        pwmgr.removeLogin(login1);
+        pwmgr.removeLogin(login1B);
 
         // Add login for the next test
         pwmgr.addLogin(login2B);
         break;
 
       case 26:
         // Check for notification popup when a form with 2 password fields (no username) is
         // submitted and there is a saved login with no username and a different password.
         is(gotUser, "null", "Checking submitted username");
-        is(gotPass, "notifyp2", "Checking submitted password");
+        is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-change");
         ok(popup, "got notification popup");
         popup.remove();
         // remove that login
         pwmgr.removeLogin(login2B);
+
+        // Add login for the next test
+        pwmgr.addLogin(login1);
+
+        break;
+
+      case 27:
+        // Check for no notification popup when a form with 2 password fields (no username) is
+        // submitted and there is a saved login with a username and the same password.
+        is(gotUser, "null", "Checking submitted username");
+        is(gotPass, "notifyp1", "Checking submitted password");
+        popup = getPopup(popupNotifications, "password-change");
+        ok(!popup, "checking for no notification popup");
+
+        // Check to make sure we updated the timestamps and use count on the
+        // existing login that was submitted for this form.
+        var logins = pwmgr.getAllLogins();
+        is(logins.length, 1, "Should only have 1 login");
+        ok(SpecialPowers.call_Instanceof(logins[0], Ci.nsILoginMetaInfo), "metainfo QI");
+        is(logins[0].timesUsed, 2, "check .timesUsed for existing login submission");
+        ok(logins[0].timeLastUsed > logins[0].timeCreated, "timeLastUsed bumped");
+        ok(logins[0].timeCreated == logins[0].timePasswordChanged, "timeChanged not updated");
+
+        // remove that login
+        pwmgr.removeLogin(login1);
+
+        // Add login for the next test
+        pwmgr.addLogin(login2);
+        break;
+
+      case 28:
+        // Check for no notification popup when a form with 2 password fields (no username) is
+        // submitted and there is a saved login with no username and the same password.
+        is(gotUser, "null", "Checking submitted username");
+        is(gotPass, "notifyp1", "Checking submitted password");
+        popup = getPopup(popupNotifications, "password-change");
+        ok(!popup, "checking for no notification popup");
+
+        // Check to make sure we updated the timestamps and use count on the
+        // existing login that was submitted for this form.
+        var logins = pwmgr.getAllLogins();
+        is(logins.length, 1, "Should only have 1 login");
+        ok(SpecialPowers.call_Instanceof(logins[0], Ci.nsILoginMetaInfo), "metainfo QI");
+        is(logins[0].timesUsed, 2, "check .timesUsed for existing login submission");
+        ok(logins[0].timeLastUsed > logins[0].timeCreated, "timeLastUsed bumped");
+        ok(logins[0].timeCreated == logins[0].timePasswordChanged, "timeChanged not updated");
+
+        // remove that login
+        pwmgr.removeLogin(login2);
         break;
 
       default:
         ok(false, "Unexpected call to checkTest for test #" + testNum);
 
     }
 
     // TODO: