Bug 1145913 - Make the username in the password notification editable. r=MattN
☠☠ backed out by c12899fa2f8a ☠ ☠
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 23 Mar 2015 16:43:16 -0700
changeset 264146 56ce9fdb62e3d166dada53e38d1a043bc87036db
parent 264145 cb3c08dcccf6d8fdef1e10fb89bcc902f222b9a3
child 264147 a8194e05e0477c62c5dd89cbf05c5665454b4a27
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
@@ -57,17 +57,17 @@
       <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-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,117 @@ 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, null);
+      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 +913,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();
+  }
+});