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 272999 e93eb477c5aa05405f43bac72566177440da2594
parent 272998 3f2af35fb884b1b50ca9d6d2a418a9f3e194f00e
child 273000 ff7b5b03fece6c5d2e3fee12cfe6311ad45be934
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske
bugs1155390
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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: