From cbf7b933f1f2236faebf897b45cd666d9e4680de Mon Sep 17 00:00:00 2001
From: Paolo Amadini <paolo.mozmail@amadzone.org>
Date: Fri, 20 Mar 2015 10:26:01 -0700
Subject: [PATCH] Bug 1143903 - Display username and password as separate
 fields in the password doorhanger. r=MattN

---
 browser/base/content/popup-notifications.inc  |   8 ++
 .../passwordmgr/nsLoginManagerPrompter.js     |  67 ++++++++----
 .../test/browser/browser_notifications.js     | 100 +++++++++++++-----
 .../passwordmgr/test/notification_common.js   |   2 +-
 .../passwordmgr/test/test_notifications.html  |   4 +-
 .../chrome/passwordmgr/passwordmgr.properties |   3 +
 6 files changed, 134 insertions(+), 50 deletions(-)

diff --git a/browser/base/content/popup-notifications.inc b/browser/base/content/popup-notifications.inc
index e4bdea72a36..4082f4a36c0 100644
--- a/browser/base/content/popup-notifications.inc
+++ b/browser/base/content/popup-notifications.inc
@@ -60,6 +60,14 @@
       </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"/>
diff --git a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
index c36e9d43325..983b18159f0 100644
--- a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -806,19 +806,13 @@ LoginManagerPrompter.prototype = {
           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
@@ -854,12 +848,28 @@ LoginManagerPrompter.prototype = {
 
       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");
@@ -1016,21 +1026,18 @@ LoginManagerPrompter.prototype = {
    *        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
@@ -1053,12 +1060,28 @@ LoginManagerPrompter.prototype = {
 
       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");
diff --git a/toolkit/components/passwordmgr/test/browser/browser_notifications.js b/toolkit/components/passwordmgr/test/browser/browser_notifications.js
index 29e503f4c5c..16efd9251a5 100644
--- 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);
 
-  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];
+/**
+ * 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",
+  }];
 
-  let promiseLogin = TestUtils.topicObserved("passwordmgr-storage-changed",
-                                             (_, data) => data == "addLogin");
-  notificationElement.button.doCommand();
-  let [login] = yield promiseLogin;
-  login.QueryInterface(Ci.nsILoginInfo);
+  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,
+      }));
+    }
 
-  Assert.equal(login.username, "username");
-  Assert.equal(login.password, "password");
+    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;
 
-  // Cleanup.
-  Services.logins.removeAllLogins();
-  gBrowser.removeTab(tab);
+      // 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);
+
+      // 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;
+
+      // 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();
+  }
 });
diff --git a/toolkit/components/passwordmgr/test/notification_common.js b/toolkit/components/passwordmgr/test/notification_common.js
index 8b774aae9b6..ca3511cc777 100644
--- a/toolkit/components/passwordmgr/test/notification_common.js
+++ b/toolkit/components/passwordmgr/test/notification_common.js
@@ -64,7 +64,7 @@ function clickPopupButton(aPopup, aButtonIndex) {
         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();
     }
diff --git a/toolkit/components/passwordmgr/test/test_notifications.html b/toolkit/components/passwordmgr/test/test_notifications.html
index 65e634a321d..492e8d865d5 100644
--- a/toolkit/components/passwordmgr/test/test_notifications.html
+++ b/toolkit/components/passwordmgr/test/test_notifications.html
@@ -347,7 +347,7 @@ function checkTest() {
         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;
@@ -360,7 +360,7 @@ function checkTest() {
         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;
diff --git a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
index db1bdc0c3b3..230a26451ca 100644
--- a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
+++ b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ -12,6 +12,9 @@ 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