Bug 1145913 - Make the username in the password notification editable. r=MattN
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 24 Mar 2015 11:18:19 -0700
changeset 264272 4944dc94d987f000842733919d9ed1cb29e18c7f
parent 264271 90e2fc63452d431b9bde6019059f869f2becba26
child 264273 a18c01416b7f14d4d582e7a4094a2398c43239ed
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
bugs1145913
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 1145913 - Make the username in the password notification editable. r=MattN
browser/base/content/popup-notifications.inc
toolkit/components/passwordmgr/nsLoginManagerPrompter.js
toolkit/components/passwordmgr/test/browser/browser_notifications.js
--- a/browser/base/content/popup-notifications.inc
+++ b/browser/base/content/popup-notifications.inc
@@ -51,17 +51,17 @@
     <popupnotification id="pointerLock-notification" hidden="true">
       <popupnotificationcontent orient="vertical" align="start">
         <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-username"/>
         <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"/>
--- a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -5,16 +5,20 @@
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 Cu.import("resource://gre/modules/SharedPromptUtils.jsm");
 
+const LoginInfo =
+      Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
+                             "nsILoginInfo", "init");
+
 /* Constants for password prompt telemetry.
  * Mirrored in mobile/android/components/LoginManagerPrompter.js */
 const PROMPT_DISPLAYED = 0;
 
 const PROMPT_ADD_OR_UPDATE = 1;
 const PROMPT_NOTNOW = 2;
 const PROMPT_NEVER = 3;
 
@@ -787,50 +791,118 @@ LoginManagerPrompter.prototype = {
    *        new password.
    * @param {string} type
    *        This is "password-save" or "password-change" depending on the
    *        original notification type. This is used for telemetry and tests.
    */
   _showLoginCaptureDoorhanger(login, type) {
     let { browser } = this._getNotifyWindow();
 
-    let msgNames = type == "password-save" ? {
+    let saveMsgNames = {
       prompt: "rememberPasswordMsgNoUsername",
       buttonLabel: "notifyBarRememberPasswordButtonText",
       buttonAccessKey: "notifyBarRememberPasswordButtonAccessKey",
-    } : {
+    };
+
+    let changeMsgNames = {
       // We reuse the existing message, even if it expects a username, until we
       // switch to the final terminology in bug 1144856.
       prompt: "updatePasswordMsg",
       buttonLabel: "notifyBarUpdateButtonText",
       buttonAccessKey: "notifyBarUpdateButtonAccessKey",
     };
 
+    let initialMsgNames = type == "password-save" ? saveMsgNames
+                                                  : changeMsgNames;
+
     let histogramName = type == "password-save" ? "PWMGR_PROMPT_REMEMBER_ACTION"
                                                 : "PWMGR_PROMPT_UPDATE_ACTION";
     let histogram = Services.telemetry.getHistogramById(histogramName);
     histogram.add(PROMPT_DISPLAYED);
 
+    let chromeDoc = browser.ownerDocument;
+
+    let currentNotification;
+
+    let updateButtonLabel = () => {
+      let foundLogins = Services.logins.findLogins({}, login.hostname,
+                                                   login.formSubmitURL,
+                                                   login.httpRealm);
+      let logins = foundLogins.filter(l => l.username == login.username);
+      let msgNames = (logins.length == 0) ? saveMsgNames : changeMsgNames;
+
+      // Update the label based on whether this will be a new login or not.
+      let label = this._getLocalizedString(msgNames.buttonLabel);
+      let accessKey = this._getLocalizedString(msgNames.buttonAccessKey);
+
+      // Update the labels for the next time the panel is opened.
+      currentNotification.mainAction.label = label;
+      currentNotification.mainAction.accessKey = accessKey;
+
+      // Update the labels in real time if the notification is displayed.
+      let element = [...currentNotification.owner.panel.childNodes]
+                    .find(n => n.notification == currentNotification);
+      if (element) {
+        element.setAttribute("buttonlabel", label);
+        element.setAttribute("buttonaccesskey", accessKey);
+      }
+    };
+
+    let writeDataToUI = () => {
+      chromeDoc.getElementById("password-notification-username")
+               .setAttribute("placeholder", usernamePlaceholder);
+      chromeDoc.getElementById("password-notification-username")
+               .setAttribute("value", login.username);
+      chromeDoc.getElementById("password-notification-password")
+               .setAttribute("value", login.password);
+      updateButtonLabel();
+    };
+
+    let readDataFromUI = () => {
+      login.username =
+        chromeDoc.getElementById("password-notification-username").value;
+      login.password =
+        chromeDoc.getElementById("password-notification-password").value;
+    };
+
+    let onUsernameInput = () => {
+      readDataFromUI();
+      updateButtonLabel();
+    };
+
+    let persistData = () => {
+      let foundLogins = Services.logins.findLogins({}, login.hostname,
+                                                   login.formSubmitURL,
+                                                   login.httpRealm);
+      let logins = foundLogins.filter(l => l.username == login.username);
+      if (logins.length == 0) {
+        // The original login we have been provided with might have its own
+        // metadata, but we don't want it propagated to the newly created one.
+        Services.logins.addLogin(new LoginInfo(login.hostname,
+                                               login.formSubmitURL,
+                                               login.httpRealm,
+                                               login.username,
+                                               login.password,
+                                               login.usernameField,
+                                               login.passwordField));
+      } else if (logins.length == 1) {
+        this._updateLogin(logins[0], login.password);
+      } else {
+        Cu.reportError("Unexpected match of multiple logins.");
+      }
+    };
+
     // The main action is the "Remember" or "Update" button.
     let mainAction = {
-      label: this._getLocalizedString(msgNames.buttonLabel),
-      accessKey: this._getLocalizedString(msgNames.buttonAccessKey),
+      label: this._getLocalizedString(initialMsgNames.buttonLabel),
+      accessKey: this._getLocalizedString(initialMsgNames.buttonAccessKey),
       callback: () => {
         histogram.add(PROMPT_ADD_OR_UPDATE);
-        let foundLogins = Services.logins.findLogins({}, login.hostname,
-                                                     login.formSubmitURL,
-                                                     login.httpRealm);
-        let logins = foundLogins.filter(l => l.username == login.username);
-        if (logins.length == 0) {
-          Services.logins.addLogin(login);
-        } else if (logins.length == 1) {
-          this._updateLogin(logins[0], login.password);
-        } else {
-          Cu.reportError("Unexpected match of multiple logins.");
-        }
+        readDataFromUI();
+        persistData();
         browser.focus();
       }
     };
 
     // Include a "Never for this site" button when saving a new password.
     let secondaryActions = type == "password-save" ? [{
       label: this._getLocalizedString("notifyBarNeverRememberButtonText"),
       accessKey: this._getLocalizedString("notifyBarNeverRememberButtonAccessKey"),
@@ -842,37 +914,42 @@ LoginManagerPrompter.prototype = {
     }] : null;
 
     let usernamePlaceholder = this._getLocalizedString("noUsernamePlaceholder");
     let displayHost = this._getShortDisplayHost(login.hostname);
 
     this._getPopupNote().show(
       browser,
       "password",
-      this._getLocalizedString(msgNames.prompt, [displayHost]),
+      this._getLocalizedString(initialMsgNames.prompt, [displayHost]),
       "password-notification-icon",
       mainAction,
       secondaryActions,
       {
         timeout: Date.now() + 10000,
         persistWhileVisible: true,
         passwordNotificationType: type,
         eventCallback: function (topic) {
-          if (topic != "showing") {
-            return false;
+          switch (topic) {
+            case "showing":
+              currentNotification = this;
+              writeDataToUI();
+              chromeDoc.getElementById("password-notification-username")
+                       .addEventListener("input", onUsernameInput);
+              break;
+            case "dismissed":
+              readDataFromUI();
+              // Fall through.
+            case "removed":
+              currentNotification = null;
+              chromeDoc.getElementById("password-notification-username")
+                       .removeEventListener("input", onUsernameInput);
+              break;
           }
-
-          let chromeDoc = this.browser.ownerDocument;
-
-          chromeDoc.getElementById("password-notification-username")
-                   .setAttribute("placeholder", usernamePlaceholder);
-          chromeDoc.getElementById("password-notification-username")
-                   .setAttribute("value", login.username);
-          chromeDoc.getElementById("password-notification-password")
-                   .setAttribute("value", login.password);
+          return false;
         },
       }
     );
   },
 
   /*
    * _showSaveLoginNotification
    *
--- a/toolkit/components/passwordmgr/test/browser/browser_notifications.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_notifications.js
@@ -76,8 +76,124 @@ add_task(function* test_save_change() {
       Assert.equal(login.username, username);
       Assert.equal(login.password, password);
     });
 
     // Clean up the database before the next test case is executed.
     Services.logins.removeAllLogins();
   }
 });
+
+/**
+ * Test changing the username inside the doorhanger notification for passwords.
+ *
+ * We have to test combination of existing and non-existing logins both for
+ * the original one from the webpage and the final one used by the dialog.
+ *
+ * We also check switching to and from empty usernames.
+ */
+add_task(function* test_edit_username() {
+  let testCases = [{
+    usernameInPage: "username",
+    usernameChangedTo: "newUsername",
+  }, {
+    usernameInPage: "username",
+    usernameInPageExists: true,
+    usernameChangedTo: "newUsername",
+  }, {
+    usernameInPage: "username",
+    usernameChangedTo: "newUsername",
+    usernameChangedToExists: true,
+  }, {
+    usernameInPage: "username",
+    usernameInPageExists: true,
+    usernameChangedTo: "newUsername",
+    usernameChangedToExists: true,
+  }, {
+    usernameInPage: "",
+    usernameChangedTo: "newUsername",
+  }, {
+    usernameInPage: "newUsername",
+    usernameChangedTo: "",
+  }, {
+    usernameInPage: "",
+    usernameChangedTo: "newUsername",
+    usernameChangedToExists: true,
+  }, {
+    usernameInPage: "newUsername",
+    usernameChangedTo: "",
+    usernameChangedToExists: true,
+  }];
+
+  for (let testCase of testCases) {
+    info("Test case: " + JSON.stringify(testCase));
+
+    // Create the pre-existing logins when needed.
+    if (testCase.usernameInPageExists) {
+      Services.logins.addLogin(LoginTestUtils.testData.formLogin({
+        hostname: "https://example.com",
+        formSubmitURL: "https://example.com",
+        username: testCase.usernameInPage,
+        password: "old password",
+      }));
+    }
+    if (testCase.usernameChangedToExists) {
+      Services.logins.addLogin(LoginTestUtils.testData.formLogin({
+        hostname: "https://example.com",
+        formSubmitURL: "https://example.com",
+        username: testCase.usernameChangedTo,
+        password: "old 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, testCase.usernameInPage,
+        function* (usernameInPage) {
+          let doc = content.document;
+          doc.getElementById("form-basic-username").value = usernameInPage;
+          doc.getElementById("form-basic-password").value = "password";
+          doc.getElementById("form-basic").submit();
+        });
+      yield promiseShown;
+
+      // Modify the username in the dialog if requested.
+      if (testCase.usernameChangedTo) {
+        document.getElementById("password-notification-username")
+                .setAttribute("value", testCase.usernameChangedTo);
+      }
+
+      // We expect a modifyLogin notification if the final username used by the
+      // dialog exists in the logins database, otherwise an addLogin one.
+      let expectModifyLogin = testCase.usernameChangedTo
+                              ? testCase.usernameChangedToExists
+                              : testCase.usernameInPageExists;
+
+      // 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 = expectModifyLogin ? "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 = expectModifyLogin ? result.QueryInterface(Ci.nsIArray)
+                                            .queryElementAt(1, Ci.nsILoginInfo)
+                                    : result.QueryInterface(Ci.nsILoginInfo);
+      Assert.equal(login.username, testCase.usernameChangedTo ||
+                                   testCase.usernameInPage);
+      Assert.equal(login.password, "password");
+    });
+
+    // Clean up the database before the next test case is executed.
+    Services.logins.removeAllLogins();
+  }
+});