Bug 1143903 - Display username and password as separate fields in the password doorhanger. r=MattN
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Fri, 20 Mar 2015 10:26:01 -0700
changeset 263465 aa2b8bc620db9e80b6c8254e654c2f7d2153f9af
parent 263464 477656ff3fc5373def775dc5daf64bbe489c6d41
child 263466 f79e4acd9172351661d55456ccc406fe08190ccf
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1143903
milestone39.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 1143903 - Display username and password as separate fields in the password doorhanger. r=MattN
browser/base/content/popup-notifications.inc
toolkit/components/passwordmgr/nsLoginManagerPrompter.js
toolkit/components/passwordmgr/test/browser/browser_notifications.js
toolkit/components/passwordmgr/test/notification_common.js
toolkit/components/passwordmgr/test/test_notifications.html
toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
--- a/browser/base/content/popup-notifications.inc
+++ b/browser/base/content/popup-notifications.inc
@@ -55,13 +55,21 @@
 
     <popupnotification id="pointerLock-notification" hidden="true">
       <popupnotificationcontent orient="vertical" align="start">
         <separator class="thin"/>
         <label id="pointerLock-cancel">&pointerLock.notification.message;</label>
       </popupnotificationcontent>
     </popupnotification>
 
+    <popupnotification id="password-notification" hidden="true">
+      <popupnotificationcontent orient="vertical">
+        <textbox id="password-notification-username" disabled="true"/>
+        <textbox id="password-notification-password" type="password"
+                 disabled="true"/>
+      </popupnotificationcontent>
+    </popupnotification>
+
 #ifdef E10S_TESTING_ONLY
     <popupnotification id="enable-e10s-notification" hidden="true">
       <popupnotificationcontent orient="vertical"/>
     </popupnotification>
 #endif
--- a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -801,29 +801,23 @@ LoginManagerPrompter.prototype = {
     var neverButtonText =
           this._getLocalizedString("notifyBarNeverRememberButtonText");
     var neverButtonAccessKey =
           this._getLocalizedString("notifyBarNeverRememberButtonAccessKey");
     var rememberButtonText =
           this._getLocalizedString("notifyBarRememberPasswordButtonText");
     var rememberButtonAccessKey =
           this._getLocalizedString("notifyBarRememberPasswordButtonAccessKey");
+    var usernamePlaceholder =
+          this._getLocalizedString("noUsernamePlaceholder");
 
     var displayHost = this._getShortDisplayHost(aLogin.hostname);
-    var notificationText;
-    if (aLogin.username) {
-      var displayUser = this._sanitizeUsername(aLogin.username);
-      notificationText  = this._getLocalizedString(
-                                  "rememberPasswordMsg",
-                                  [displayUser, displayHost]);
-    } else {
-      notificationText  = this._getLocalizedString(
+    var notificationText = this._getLocalizedString(
                                   "rememberPasswordMsgNoUsername",
                                   [displayHost]);
-    }
 
     // The callbacks in |buttons| have a closure to access the variables
     // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
     // without a getService() call.
     var pwmgr = this._pwmgr;
     let promptHistogram = Services.telemetry.getHistogramById("PWMGR_PROMPT_REMEMBER_ACTION");
 
     // Notification is a PopupNotification
@@ -849,22 +843,38 @@ LoginManagerPrompter.prototype = {
             pwmgr.setLoginSavingEnabled(aLogin.hostname, false);
             browser.focus();
           }
         }
       ];
 
       var { browser } = this._getNotifyWindow();
 
+      let eventCallback = function (topic) {
+        if (topic != "showing") {
+          return false;
+        }
+
+        let chromeDoc = this.browser.ownerDocument;
+
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("placeholder", usernamePlaceholder);
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("value", aLogin.username);
+        chromeDoc.getElementById("password-notification-password")
+                 .setAttribute("value", aLogin.password);
+      };
+
       aNotifyObj.show(browser, "password", notificationText,
                       "password-notification-icon", mainAction,
                       secondaryActions,
                       { timeout: Date.now() + 10000,
                         persistWhileVisible: true,
-                        passwordNotificationType: "password-save" });
+                        passwordNotificationType: "password-save",
+                        eventCallback });
     } else {
       var notNowButtonText =
             this._getLocalizedString("notifyBarNotNowButtonText");
       var notNowButtonAccessKey =
             this._getLocalizedString("notifyBarNotNowButtonAccessKey");
       var buttons = [
         // "Remember" button
         {
@@ -1011,31 +1021,28 @@ LoginManagerPrompter.prototype = {
    * _showChangeLoginNotification
    *
    * Shows the Change Password notification bar or popup notification.
    *
    * @param aNotifyObj
    *        A notification box or a popup notification.
    */
   _showChangeLoginNotification : function (aNotifyObj, aOldLogin, aNewPassword) {
-    var notificationText;
-    if (aOldLogin.username) {
-      var displayUser = this._sanitizeUsername(aOldLogin.username);
-      notificationText  = this._getLocalizedString(
-                                    "updatePasswordMsg",
-                                    [displayUser]);
-    } else {
-      notificationText  = this._getLocalizedString(
-                                    "updatePasswordMsgNoUser");
-    }
-
     var changeButtonText =
           this._getLocalizedString("notifyBarUpdateButtonText");
     var changeButtonAccessKey =
           this._getLocalizedString("notifyBarUpdateButtonAccessKey");
+    var usernamePlaceholder =
+          this._getLocalizedString("noUsernamePlaceholder");
+
+    // We reuse the existing message, even if it expects a username, until we
+    // switch to the final terminology in bug 1144856.
+    var displayHost = this._getShortDisplayHost(aOldLogin.hostname);
+    var notificationText = this._getLocalizedString("updatePasswordMsg",
+                                                    [displayHost]);
 
     // The callbacks in |buttons| have a closure to access the variables
     // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
     // without a getService() call.
     var self = this;
 
     let promptHistogram = Services.telemetry.getHistogramById("PWMGR_PROMPT_UPDATE_ACTION");
     // Notification is a PopupNotification
@@ -1048,22 +1055,38 @@ LoginManagerPrompter.prototype = {
         callback:  function(aNotifyObj, aButton) {
           self._updateLogin(aOldLogin, aNewPassword);
           promptHistogram.add(PROMPT_UPDATE);
         }
       };
 
       var { browser } = this._getNotifyWindow();
 
+      let eventCallback = function (topic) {
+        if (topic != "showing") {
+          return false;
+        }
+
+        let chromeDoc = this.browser.ownerDocument;
+
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("placeholder", usernamePlaceholder);
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("value", aOldLogin.username);
+        chromeDoc.getElementById("password-notification-password")
+                 .setAttribute("value", aNewPassword);
+      };
+
       Services.telemetry.getHistogramById("PWMGR_PROMPT_UPDATE_ACTION").add(PROMPT_DISPLAYED);
       aNotifyObj.show(browser, "password", notificationText,
                       "password-notification-icon", mainAction,
                       null, { timeout: Date.now() + 10000,
                               persistWhileVisible: true,
-                              passwordNotificationType: "password-change" });
+                              passwordNotificationType: "password-change",
+                              eventCallback });
     } else {
       var dontChangeButtonText =
             this._getLocalizedString("notifyBarDontChangeButtonText");
       var dontChangeButtonAccessKey =
             this._getLocalizedString("notifyBarDontChangeButtonAccessKey");
       var buttons = [
         // "Yes" button
         {
--- a/toolkit/components/passwordmgr/test/browser/browser_notifications.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_notifications.js
@@ -1,33 +1,83 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-add_task(function* test_save() {
-  let tab = gBrowser.addTab("https://example.com/browser/toolkit/components/" +
-                            "passwordmgr/test/browser/form_basic.html");
-  let browser = tab.linkedBrowser;
-  yield BrowserTestUtils.browserLoaded(browser);
-  gBrowser.selectedTab = tab;
+Cu.import("resource://testing-common/LoginTestUtils.jsm", this);
+
+/**
+ * Test that the doorhanger notification for password saving is populated with
+ * the correct values in various password capture cases.
+ */
+add_task(function* test_save_change() {
+  let testCases = [{
+    username: "username",
+    password: "password",
+  }, {
+    username: "",
+    password: "password",
+  }, {
+    username: "username",
+    oldPassword: "password",
+    password: "newPassword",
+  }, {
+    username: "",
+    oldPassword: "password",
+    password: "newPassword",
+  }];
+
+  for (let { username, oldPassword, password } of testCases) {
+    // Add a login for the origin of the form if testing a change notification.
+    if (oldPassword) {
+      Services.logins.addLogin(LoginTestUtils.testData.formLogin({
+        hostname: "https://example.com",
+        formSubmitURL: "https://example.com",
+        username,
+        password: oldPassword,
+      }));
+    }
 
-  let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
-                                                   "Shown");
-  yield ContentTask.spawn(browser, null, function* () {
-    content.document.getElementById("form-basic-username").value = "username";
-    content.document.getElementById("form-basic-password").value = "password";
-    content.document.getElementById("form-basic").submit();
-  });
-  yield promiseShown;
-  let notificationElement = PopupNotifications.panel.childNodes[0];
+    yield BrowserTestUtils.withNewTab({
+      gBrowser,
+      url: "https://example.com/browser/toolkit/components/" +
+           "passwordmgr/test/browser/form_basic.html",
+    }, function* (browser) {
+      // Submit the form in the content page with the credentials from the test
+      // case. This will cause the doorhanger notification to be displayed.
+      let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
+                                                       "Shown");
+      yield ContentTask.spawn(browser, { username, password },
+        function* ({ username, password }) {
+          let doc = content.document;
+          doc.getElementById("form-basic-username").value = username;
+          doc.getElementById("form-basic-password").value = password;
+          doc.getElementById("form-basic").submit();
+        });
+      yield promiseShown;
 
-  let promiseLogin = TestUtils.topicObserved("passwordmgr-storage-changed",
-                                             (_, data) => data == "addLogin");
-  notificationElement.button.doCommand();
-  let [login] = yield promiseLogin;
-  login.QueryInterface(Ci.nsILoginInfo);
+      // Check the actual content of the popup notification.
+      Assert.equal(document.getElementById("password-notification-username")
+                           .getAttribute("value"), username);
+      Assert.equal(document.getElementById("password-notification-password")
+                           .getAttribute("value"), password);
 
-  Assert.equal(login.username, "username");
-  Assert.equal(login.password, "password");
+      // Simulate the action on the notification to request the login to be
+      // saved, and wait for the data to be updated or saved based on the type
+      // of operation we expect.
+      let expectedNotification = oldPassword ? "modifyLogin" : "addLogin";
+      let promiseLogin = TestUtils.topicObserved("passwordmgr-storage-changed",
+                         (_, data) => data == expectedNotification);
+      let notificationElement = PopupNotifications.panel.childNodes[0];
+      notificationElement.button.doCommand();
+      let [result] = yield promiseLogin;
 
-  // Cleanup.
-  Services.logins.removeAllLogins();
-  gBrowser.removeTab(tab);
+      // Check that the values in the database match the expected values.
+      let login = oldPassword ? result.QueryInterface(Ci.nsIArray)
+                                      .queryElementAt(1, Ci.nsILoginInfo)
+                              : result.QueryInterface(Ci.nsILoginInfo);
+      Assert.equal(login.username, username);
+      Assert.equal(login.password, password);
+    });
+
+    // Clean up the database before the next test case is executed.
+    Services.logins.removeAllLogins();
+  }
 });
--- a/toolkit/components/passwordmgr/test/notification_common.js
+++ b/toolkit/components/passwordmgr/test/notification_common.js
@@ -59,17 +59,17 @@ function clickPopupButton(aPopup, aButto
     ok(notifications.length > 0, "at least one notification displayed");
     ok(true, notifications.length + " notifications");
     var notification = notifications[0];
 
     if (aButtonIndex == 0) {
         ok(true, "Triggering main action");
         notification.button.doCommand();
     } else if (aButtonIndex <= aPopup.secondaryActions.length) {
-        var index = aButtonIndex - 1;
+        var index = aButtonIndex;
         ok(true, "Triggering secondary action " + index);
         notification.childNodes[index].doCommand();
     }
 }
 
 const kRememberButton = 0;
 const kNeverButton = 1;
 
--- a/toolkit/components/passwordmgr/test/test_notifications.html
+++ b/toolkit/components/passwordmgr/test/test_notifications.html
@@ -342,30 +342,30 @@ function checkTest() {
       case 21:
         // Check text on a user+pass notification popup
         is(gotUser, "notifyu1", "Checking submitted username");
         is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-save");
         ok(popup, "got notification popup");
         // Check the text, which comes from the localized saveLoginText string.
         notificationText = popup.message;
-        expectedText = /^Would you like to remember the password for \"notifyu1\" on example.org\?$/;
+        expectedText = /^Would you like to remember the password on example.org\?$/;
         ok(expectedText.test(notificationText), "Checking text: " + notificationText);
         popup.remove();
         break;
 
       case 22:
         // Check text on a user+pass notification popup, username is really long
         is(gotUser, "nowisthetimeforallgoodmentocometotheaidoftheircountry", "Checking submitted username");
         is(gotPass, "notifyp1", "Checking submitted password");
         popup = getPopup(popupNotifications, "password-save");
         ok(popup, "got notification popup");
         // Check the text, which comes from the localized saveLoginText string.
         notificationText = popup.message;
-        expectedText = /^Would you like to remember the password for \"nowisthetimeforallgoodmentocom[^e]\" on example.org\?$/;
+        expectedText = /^Would you like to remember the password on example.org\?$/;
         ok(expectedText.test(notificationText), "Checking text: " + notificationText);
         popup.remove();
         break;
 
       case 23:
         // Check text on a pass-only notification popup
         is(gotUser, "null",     "Checking submitted username");
         is(gotPass, "notifyp1", "Checking submitted password");
--- a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
+++ b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ -7,16 +7,19 @@ rememberPassword = Use Password Manager 
 savePasswordTitle = Confirm
 # LOCALIZATION NOTE (rememberPasswordMsg):
 # 1st string is the username for the login, 2nd is the login's hostname. 
 # Note that long usernames may be truncated.
 rememberPasswordMsg = Would you like to remember the password for "%1$S" on %2$S?
 # LOCALIZATION NOTE (rememberPasswordMsgNoUsername):
 # String is the login's hostname.
 rememberPasswordMsgNoUsername = Would you like to remember the password on %S?
+# LOCALIZATION NOTE (noUsernamePlaceholder):
+# This is displayed in place of the username when it is missing.
+noUsernamePlaceholder=No username
 notNowButtonText = &Not Now
 notifyBarNotNowButtonText = Not Now
 notifyBarNotNowButtonAccessKey = N
 neverForSiteButtonText = Ne&ver for This Site
 notifyBarNeverRememberButtonText = Never Remember Password for This Site
 notifyBarNeverRememberButtonAccessKey = e
 rememberButtonText = &Remember
 notifyBarRememberPasswordButtonText = Remember Password