Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

toolkit/components/passwordmgr/content/passwordManager.js

@@ -5,7 +5,6 @@
 /*** =================== SAVED SIGNONS CODE =================== ***/
 
 var kSignonBundle;
-var showingPasswords = false;
 var dateFormatter = new Intl.DateTimeFormat(undefined,
                       { day: "numeric", month: "short", year: "numeric" });
 var dateAndTimeFormatter = new Intl.DateTimeFormat(undefined,
@@ -14,8 +13,6 @@ var dateAndTimeFormatter = new Intl.DateTimeFormat(undefined,
 
 function SignonsStartup() {
   kSignonBundle = document.getElementById("signonBundle");
-  document.getElementById("togglePasswords").label = kSignonBundle.getString("showPasswords");
-  document.getElementById("togglePasswords").accessKey = kSignonBundle.getString("showPasswordsAccessKey");
   document.getElementById("signonsIntro").textContent = kSignonBundle.getString("loginsDescriptionAll");
 
   let treecols = document.getElementsByTagName("treecols")[0];
@@ -79,9 +76,14 @@ var signonsTreeView = {
     }
   },
   isEditable : function(row, col) {
-    if (col.id == "userCol" || col.id == "passwordCol") {
+    if (col.id == "userCol") {
       return true;
     }
+
+    if (col.id == "passwordCol") {
+      return masterPasswordLogin();
+    }
+
     return false;
   },
   isSeparator : function(index) { return false; },
@@ -128,7 +130,8 @@ function LoadSignons() {
   try {
     signons = passwordmanager.getAllLogins();
   } catch (e) {
-    signons = [];
+    window.close();
+    return;
   }
   signons.forEach(login => login.QueryInterface(Components.interfaces.nsILoginMetaInfo));
   signonsTreeView.rowCount = signons.length;
@@ -143,13 +146,10 @@ function LoadSignons() {
 
   // disable "remove all signons" button if there are no signons
   var element = document.getElementById("removeAllSignons");
-  var toggle = document.getElementById("togglePasswords");
   if (signons.length == 0) {
     element.setAttribute("disabled","true");
-    toggle.setAttribute("disabled","true");
   } else {
     element.removeAttribute("disabled");
-    toggle.removeAttribute("disabled");
   }
 
   return true;
@@ -191,34 +191,6 @@ function DeleteAllSignons() {
   Services.telemetry.getHistogramById("PWMGR_MANAGE_DELETED_ALL").add(1);
 }
 
-function TogglePasswordVisible() {
-  if (showingPasswords || masterPasswordLogin(AskUserShowPasswords)) {
-    showingPasswords = !showingPasswords;
-    document.getElementById("togglePasswords").label = kSignonBundle.getString(showingPasswords ? "hidePasswords" : "showPasswords");
-    document.getElementById("togglePasswords").accessKey = kSignonBundle.getString(showingPasswords ? "hidePasswordsAccessKey" : "showPasswordsAccessKey");
-    document.getElementById("passwordCol").hidden = !showingPasswords;
-    _filterPasswords();
-  }
-
-  // Notify observers that the password visibility toggling is
-  // completed.  (Mostly useful for tests)
-  Components.classes["@mozilla.org/observer-service;1"]
-            .getService(Components.interfaces.nsIObserverService)
-            .notifyObservers(null, "passwordmgr-password-toggle-complete", null);
-  Services.telemetry.getHistogramById("PWMGR_MANAGE_VISIBILITY_TOGGLED").add(showingPasswords);
-}
-
-function AskUserShowPasswords() {
-  var prompter = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].getService(Components.interfaces.nsIPromptService);
-  var dummy = { value: false };
-
-  // Confirm the user wants to display passwords
-  return prompter.confirmEx(window,
-          null,
-          kSignonBundle.getString("noMasterPasswordPrompt"), prompter.STD_YES_NO_BUTTONS,
-          null, null, null, null, dummy) == 0;    // 0=="Yes" button
-}
-
 function FinalizeSignonDeletions(syncNeeded) {
   for (var s=0; s<deletedSignons.length; s++) {
     passwordmanager.removeLogin(deletedSignons[s]);
@@ -332,7 +304,7 @@ function SignonMatchesFilter(aSignon, aFilterValue) {
   if (aSignon.httpRealm &&
       aSignon.httpRealm.toLowerCase().indexOf(aFilterValue) != -1)
     return true;
-  if (showingPasswords && aSignon.password &&
+  if (Services.logins.isLoggedIn && aSignon.password &&
       aSignon.password.toLowerCase().indexOf(aFilterValue) != -1)
     return true;
 
@@ -388,9 +360,8 @@ function _filterPasswords()
 }
 
 function CopyPassword() {
-  // Don't copy passwords if we aren't already showing the passwords & a master
-  // password hasn't been entered.
-  if (!showingPasswords && !masterPasswordLogin())
+  // Don't copy passwords if a master password hasn't been entered.
+  if (!masterPasswordLogin())
     return;
   // Copy selected signon's password to clipboard
   var clipboard = Components.classes["@mozilla.org/widget/clipboardhelper;1"].
@@ -443,13 +414,7 @@ function UpdateContextMenu() {
 
   menuItems.get("context-editusername").removeAttribute("disabled");
   menuItems.get("context-copypassword").removeAttribute("disabled");
-
-  // Disable "Edit Password" if the password column isn't showing.
-  if (!document.getElementById("passwordCol").hidden) {
-    menuItems.get("context-editpassword").removeAttribute("disabled");
-  } else {
-    menuItems.get("context-editpassword").setAttribute("disabled", "true");
-  }
+  menuItems.get("context-editpassword").removeAttribute("disabled");
 }
 
 function masterPasswordLogin(noPasswordCallback) {

toolkit/components/passwordmgr/content/passwordManager.xul

@@ -82,9 +82,8 @@
                  data-field-name="username" persist="width"/>
         <splitter class="tree-splitter"/>
         <treecol id="passwordCol" label="&treehead.password.label;" flex="15"
-                 ignoreincolumnpicker="true"
-                 data-field-name="password" persist="width"
-                 hidden="true"/>
+                 data-field-name="password" persist="width hidden"
+                 type="password"/>
         <splitter class="tree-splitter"/>
         <treecol id="timeCreatedCol" label="&treehead.timeCreated.label;" flex="10"
                  data-field-name="timeCreated" persist="width hidden"
@@ -117,8 +116,6 @@
               label="&import.label;"
               oncommand="OpenMigrator();"/>
 #endif
-      <button id="togglePasswords"
-              oncommand="TogglePasswordVisible();"/>
     </hbox>
   </vbox>
   <hbox align="end">

toolkit/components/passwordmgr/test/browser/browser_passwordmgr_contextmenu.js

@@ -50,7 +50,7 @@ function test() {
             assertMenuitemEnabled("copyusername", false, "empty username");
             assertMenuitemEnabled("editusername", true);
             assertMenuitemEnabled("copypassword", true);
-            assertMenuitemEnabled("editpassword", false, "password column hidden");
+            assertMenuitemEnabled("editpassword", true);
 
             info("Clear the selection");
             selection.clearSelection();
@@ -61,7 +61,6 @@ function test() {
 
             info("Select the third row and making the password column visible");
             selection.select(2);
-            doc.getElementById("passwordCol").hidden = false;
             assertMenuitemEnabled("copyusername", true);
             assertMenuitemEnabled("editusername", true);
             assertMenuitemEnabled("copypassword", true);
@@ -80,7 +79,6 @@ function test() {
             Services.ww.registerNotification(function (aSubject, aTopic, aData) {
                 Services.ww.unregisterNotification(arguments.callee);
                 Services.logins.removeAllLogins();
-                doc.getElementById("passwordCol").hidden = true;
                 finish();
             });
             pwmgrdlg.close();

toolkit/components/passwordmgr/test/browser/browser_passwordmgr_editing.js

@@ -32,11 +32,6 @@ function synthesizeDblClickOnCell(aTree, column, row) {
                              aTree.ownerDocument.defaultView);
 }
 
-function* togglePasswords() {
-  pwmgrdlg.document.querySelector("#togglePasswords").doCommand();
-  yield new Promise(resolve => waitForFocus(resolve, pwmgrdlg));
-}
-
 function* editUsernamePromises(site, oldUsername, newUsername) {
   is(Services.logins.findLogins({}, site, "", "").length, 1, "Correct login found");
   let login = Services.logins.findLogins({}, site, "", "")[0];
@@ -114,9 +109,7 @@ add_task(function* test_edit_multiple_logins() {
   function* testLoginChange(site, oldUsername, oldPassword, newUsername, newPassword) {
     addLogin(site, oldUsername, oldPassword);
     yield* editUsernamePromises(site, oldUsername, newUsername);
-    yield* togglePasswords();
     yield* editPasswordPromises(site, oldPassword, newPassword);
-    yield* togglePasswords();
   }
 
   yield* testLoginChange("http://c.tn/", "userC", "passC", "usernameC", "passwordC");

toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js

@@ -67,35 +67,6 @@ function test() {
         let userCol = doc.getElementById("userCol");
         let passwordCol = doc.getElementById("passwordCol");
 
-        let toggleCalls = 0;
-        function toggleShowPasswords(func) {
-            let toggleButton = doc.getElementById("togglePasswords");
-            let showMode = (toggleCalls++ % 2) == 0;
-
-            // only watch for a confirmation dialog every other time being called
-            if (showMode) {
-                Services.ww.registerNotification(function (aSubject, aTopic, aData) {
-                    if (aTopic == "domwindowclosed")
-                        Services.ww.unregisterNotification(arguments.callee);
-                    else if (aTopic == "domwindowopened") {
-                        let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
-                        SimpleTest.waitForFocus(function() {
-                            EventUtils.sendKey("RETURN", win);
-                        }, win);
-                    }
-                });
-            }
-
-            Services.obs.addObserver(function (aSubject, aTopic, aData) {
-                if (aTopic == "passwordmgr-password-toggle-complete") {
-                    Services.obs.removeObserver(arguments.callee, aTopic);
-                    func();
-                }
-            }, "passwordmgr-password-toggle-complete", false);
-
-            EventUtils.synthesizeMouse(toggleButton, 1, 1, {}, win);
-        }
-
         function clickCol(col) {
             EventUtils.synthesizeMouse(col, 20, 1, {}, win);
             setTimeout(runNextTest, 0);
@@ -202,7 +173,6 @@ function test() {
             }
         }
 
-        // Toggle Show Passwords to display Password column, then start tests
-        toggleShowPasswords(runNextTest);
+        runNextTest();
     }
 }

toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js

@@ -57,57 +57,25 @@ function test() {
         // Prepare a set of tests
         //   filter: the text entered in the filter search box
         //   count: the number of logins which should match the respective filter
-        //   count2: the number of logins which should match the respective filter
-        //           if the passwords are being shown as well
-        //   Note: if a test doesn't have count2 set, count is used instead.
         let tests = [
-            {filter: "pass", count: 0, count2: 4},
+            {filter: "pass", count: 4},
             {filter: "", count: 10}, // test clearing the filter
             {filter: "moz", count: 7},
             {filter: "mozi", count: 7},
             {filter: "mozil", count: 7},
             {filter: "mozill", count: 7},
             {filter: "mozilla", count: 7},
-            {filter: "mozilla.com", count: 1, count2: 2},
+            {filter: "mozilla.com", count: 2},
             {filter: "user", count: 4},
             {filter: "user ", count: 1},
             {filter: " user", count: 2},
             {filter: "http", count: 10},
             {filter: "https", count: 1},
-            {filter: "secret", count: 0, count2: 2},
+            {filter: "secret", count: 2},
             {filter: "secret!", count: 0},
         ];
 
-        let toggleCalls = 0;
-        function toggleShowPasswords(func) {
-            let toggleButton = doc.getElementById("togglePasswords");
-            let showMode = (toggleCalls++ % 2) == 0;
-
-            // only watch for a confirmation dialog every other time being called
-            if (showMode) {
-                Services.ww.registerNotification(function (aSubject, aTopic, aData) {
-                    if (aTopic == "domwindowclosed")
-                        Services.ww.unregisterNotification(arguments.callee);
-                    else if (aTopic == "domwindowopened") {
-                        let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
-                        SimpleTest.waitForFocus(function() {
-                            EventUtils.sendKey("RETURN", win);
-                        }, win);
-                    }
-                });
-            }
-
-            Services.obs.addObserver(function (aSubject, aTopic, aData) {
-                if (aTopic == "passwordmgr-password-toggle-complete") {
-                    Services.obs.removeObserver(arguments.callee, aTopic);
-                    func();
-                }
-            }, "passwordmgr-password-toggle-complete", false);
-
-            EventUtils.synthesizeMouse(toggleButton, 1, 1, {}, win);
-        }
-
-        function runTests(mode, endFunction) {
+        function runTests(endFunction) {
             let testCounter = 0;
 
             function setFilter(string) {
@@ -116,29 +84,7 @@ function test() {
             }
 
             function runOneTest(test) {
-                function tester() {
-                    is(view.rowCount, expected, expected + " logins should match '" + test.filter + "'");
-                }
-
-                let expected;
-                switch (mode) {
-                case 1: // without showing passwords
-                    expected = test.count;
-                    break;
-                case 2: // showing passwords
-                    expected = ("count2" in test) ? test.count2 : test.count;
-                    break;
-                case 3: // toggle
-                    expected = test.count;
-                    tester();
-                    toggleShowPasswords(function () {
-                        expected = ("count2" in test) ? test.count2 : test.count;
-                        tester();
-                        toggleShowPasswords(proceed);
-                    });
-                    return;
-                }
-                tester();
+                is(view.rowCount, test.count, test.count + " logins should match '" + test.filter + "'");
                 proceed();
             }
 
@@ -160,19 +106,7 @@ function test() {
         }
 
         function step1() {
-            runTests(1, step2);
-        }
-
-        function step2() {
-            toggleShowPasswords(function() {
-                runTests(2, step3);
-            });
-        }
-
-        function step3() {
-            toggleShowPasswords(function() {
-                runTests(3, lastStep);
-            });
+            runTests(lastStep);
         }
 
         function lastStep() {

toolkit/components/telemetry/Histograms.json

@@ -8642,11 +8642,6 @@
     "kind": "count",
     "description": "Reports the column that logins are sorted by"
   },
-  "PWMGR_MANAGE_VISIBILITY_TOGGLED": {
-    "expires_in_version": "never",
-    "kind": "boolean",
-    "description": "Whether the visibility of passwords was toggled (0=Hide, 1=Show)"
-  },
   "PWMGR_NUM_PASSWORDS_PER_HOSTNAME": {
     "expires_in_version": "never",
     "kind": "linear",

toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties

@@ -47,11 +47,6 @@ notifyBarUpdateButtonAccessKey = U
 notifyBarDontChangeButtonText = Don't Change
 notifyBarDontChangeButtonAccessKey = D
 userSelectText = Please confirm which user you are changing the password for
-hidePasswords=Hide Passwords
-hidePasswordsAccessKey=P
-showPasswords=Show Passwords
-showPasswordsAccessKey=P
-noMasterPasswordPrompt=Are you sure you wish to show your passwords?
 removeAllPasswordsPrompt=Are you sure you wish to remove all passwords?
 removeAllPasswordsTitle=Remove all passwords
 removeLoginPrompt=Are you sure you wish to remove this login?