Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN a=sylvestre
authorBernardo P. Rittmeyer <bernardo@rittme.com>
Wed, 14 Oct 2015 17:22:21 -0400
changeset 289559 6398da8b9482
parent 289558 db9d3e806685
child 289560 b0aa8d67c934
push id5188
push usermozilla@noorenberghe.ca
push date2015-10-15 17:59 +0000
treeherdermozilla-beta@645e5fe8f354 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, sylvestre
bugs1192081
milestone42.0
Bug 1192081 - Changed createFromPasswordField to also create FormLikes from username fields. r=MattN a=sylvestre
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/test_formless_submit.html
toolkit/components/passwordmgr/test/unit/test_getFormFields.js
toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -313,16 +313,42 @@ this.LoginHelper = {
     } else {
       window.openDialog("chrome://passwordmgr/content/passwordManager.xul",
                         "Toolkit:PasswordManager", "",
                         {filterString : filterString});
     }
   },
 
   /**
+   * Checks if a field type is username compatible.
+   *
+   * @param {Element} element
+   *                  the field we want to check.
+   *
+   * @returns {Boolean} true if the field type is one
+   *                    of the username types.
+   */
+  isUsernameFieldType(element) {
+    if (!(element instanceof Ci.nsIDOMHTMLInputElement))
+      return false;
+
+    let fieldType = (element.hasAttribute("type") ?
+                     element.getAttribute("type").toLowerCase() :
+                     element.type);
+    if (fieldType == "text"  ||
+        fieldType == "email" ||
+        fieldType == "url"   ||
+        fieldType == "tel"   ||
+        fieldType == "number") {
+      return true;
+    }
+    return false;
+  },
+
+  /**
    * Add the login to the password manager if a similar one doesn't already exist. Merge it
    * otherwise with the similar existing ones.
    * @param {Object} loginData - the data about the login that needs to be added.
    */
   maybeImportLogin(loginData) {
     // create a new login
     let login = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
     login.init(loginData.hostname,
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -289,17 +289,17 @@ var LoginManagerContent = {
     }
 
     let pwField = event.target;
     if (pwField.form) {
       // Handled by onDOMFormHasPassword which is already throttled.
       return;
     }
 
-    let formLike = FormLikeFactory.createFromPasswordField(pwField);
+    let formLike = FormLikeFactory.createFromField(pwField);
     log("onDOMInputPasswordAdded:", pwField, formLike);
 
     let deferredTask = this._deferredPasswordAddedTasksByRootElement.get(formLike.rootElement);
     if (!deferredTask) {
       log("Creating a DeferredTask to call _fetchLoginsFromParentAndFillForm soon");
       this._formLikeByRootElement.set(formLike.rootElement, formLike);
 
       deferredTask = new DeferredTask(function* deferredInputProcessing() {
@@ -470,17 +470,17 @@ var LoginManagerContent = {
     let form = topState.loginFormForFill;
     let clobberUsername = true;
     let options = {
       inputElement,
     };
 
     // If we have a target input, fills it's form.
     if (inputElement) {
-      form = FormLikeFactory.createFromPasswordField(inputElement);
+      form = FormLikeFactory.createFromField(inputElement);
       clobberUsername = false;
     }
     this._fillForm(form, true, clobberUsername, true, true, loginsFound, recipes, options);
   },
 
   loginsFound: function({ form, loginsFound, recipes }) {
     let doc = form.ownerDocument;
     let autofillForm = gAutofillForms && !PrivateBrowsingUtils.isContentWindowPrivate(doc.defaultView);
@@ -501,17 +501,17 @@ var LoginManagerContent = {
       return;
 
     var acInputField = event.target;
 
     // This is probably a bit over-conservatative.
     if (!(acInputField.ownerDocument instanceof Ci.nsIDOMHTMLDocument))
       return;
 
-    if (!this._isUsernameFieldType(acInputField))
+    if (!LoginHelper.isUsernameFieldType(acInputField))
       return;
 
     var acForm = acInputField.form; // XXX: Bug 1173583 - This doesn't work outside of <form>.
     if (!acForm)
       return;
 
     // If the username is blank, bail out now -- we don't want
     // fillForm() to try filling in a login without a username
@@ -578,63 +578,49 @@ var LoginManagerContent = {
     } else if (pwFields.length > 3) {
       log("(form ignored -- too many password fields. [ got ", pwFields.length, "])");
       return null;
     }
 
     return pwFields;
   },
 
-  _isUsernameFieldType: function(element) {
-    if (!(element instanceof Ci.nsIDOMHTMLInputElement))
-      return false;
-
-    let fieldType = (element.hasAttribute("type") ?
-                     element.getAttribute("type").toLowerCase() :
-                     element.type);
-    if (fieldType == "text"  ||
-        fieldType == "email" ||
-        fieldType == "url"   ||
-        fieldType == "tel"   ||
-        fieldType == "number") {
-      return true;
-    }
-    return false;
-  },
-
-
   /**
    * Returns the username and password fields found in the form.
    * Can handle complex forms by trying to figure out what the
    * relevant fields are.
    *
    * @param {FormLike} form
    * @param {bool} isSubmission
    * @param {Set} recipes
    * @return {Array} [usernameField, newPasswordField, oldPasswordField]
    *
    * usernameField may be null.
    * newPasswordField will always be non-null.
    * oldPasswordField may be null. If null, newPasswordField is just
    * "theLoginField". If not null, the form is apparently a
    * change-password field, with oldPasswordField containing the password
    * that is being changed.
+   *
+   * Note that even though we can create a FormLike from a text field,
+   * this method will only return a non-null usernameField if the
+   * FormLike has a password field.
    */
   _getFormFields : function (form, isSubmission, recipes) {
     var usernameField = null;
     var pwFields = null;
     var fieldOverrideRecipe = LoginRecipesContent.getFieldOverrides(recipes, form);
     if (fieldOverrideRecipe) {
       var pwOverrideField = LoginRecipesContent.queryLoginField(
         form,
         fieldOverrideRecipe.passwordSelector
       );
       if (pwOverrideField) {
         // The field from the password override may be in a different FormLike.
-        let formLike = FormLikeFactory.createFromPasswordField(pwOverrideField);
+        let formLike = FormLikeFactory.createFromField(pwOverrideField);
         pwFields = [{
           index   : [...formLike.elements].indexOf(pwOverrideField),
           element : pwOverrideField,
         }];
       }
 
       var usernameOverrideField = LoginRecipesContent.queryLoginField(
         form,
@@ -657,17 +643,17 @@ var LoginManagerContent = {
 
     if (!usernameField) {
       // Locate the username field in the form by searching backwards
       // from the first password field, assume the first text field is the
       // username. We might not find a username field if the user is
       // already logged in to the site.
       for (var i = pwFields[0].index - 1; i >= 0; i--) {
         var element = form.elements[i];
-        if (this._isUsernameFieldType(element)) {
+        if (LoginHelper.isUsernameFieldType(element)) {
           usernameField = element;
           break;
         }
       }
     }
 
     if (!usernameField)
       log("(form -- no username field found)");
@@ -1087,17 +1073,16 @@ var LoginUtils = {
     var uriString = form.action;
 
     // A blank or missing action submits to where it came from.
     if (uriString == "")
       uriString = form.baseURI; // ala bug 297761
 
     return this._getPasswordOrigin(uriString, true);
   },
-
 };
 
 // nsIAutoCompleteResult implementation
 function UserAutoCompleteResult (aSearchString, matchingLogins) {
   function loginSort(a,b) {
     var userA = a.username.toLowerCase();
     var userB = b.username.toLowerCase();
 
@@ -1217,40 +1202,40 @@ let FormLikeFactory = {
       formLike[prop] = aForm[prop];
     }
 
     this._addToJSONProperty(formLike);
     return formLike;
   },
 
   /**
-   * Create a FormLike object from an <input type=password>.
+   * Create a FormLike object from a password or username field.
    *
-   * If the <input> is in a <form>, construct the FormLike from the form.
+   * If the field is in a <form>, construct the FormLike from the form.
    * Otherwise, create a FormLike with a rootElement (wrapper) according to
    * heuristics. Currently all <input> not in a <form> are one FormLike but this
    * shouldn't be relied upon as the heuristics may change to detect multiple
    * "forms" (e.g. registration and login) on one page with a <form>.
    *
-   * @param {HTMLInputElement} aPasswordField - a password field in a document
+   * @param {HTMLInputElement} aField - a password or username field in a document
    * @return {FormLike}
-   * @throws Error if aPasswordField isn't a password input in a document
+   * @throws Error if aField isn't a password or username field in a document
    */
-  createFromPasswordField(aPasswordField) {
-    if (!(aPasswordField instanceof Ci.nsIDOMHTMLInputElement) ||
-        aPasswordField.type != "password" ||
-        !aPasswordField.ownerDocument) {
-      throw new Error("createFromPasswordField requires a password field in a document");
+  createFromField(aField) {
+    if (!(aField instanceof Ci.nsIDOMHTMLInputElement) ||
+        (aField.type != "password" && !LoginHelper.isUsernameFieldType(aField)) ||
+        !aField.ownerDocument) {
+      throw new Error("createFromField requires a password or username field in a document");
     }
 
-    if (aPasswordField.form) {
-      return this.createFromForm(aPasswordField.form);
+    if (aField.form) {
+      return this.createFromForm(aField.form);
     }
 
-    let doc = aPasswordField.ownerDocument;
+    let doc = aField.ownerDocument;
     log("Created non-form FormLike for rootElement:", doc.documentElement);
     let formLike = {
       action: LoginUtils._getPasswordOrigin(doc.baseURI),
       autocomplete: "on",
       // Exclude elements inside the rootElement that are already in a <form> as
       // they will be handled by their own FormLike.
       elements: [for (el of doc.documentElement.querySelectorAll("input")) if (!el.form) el],
       ownerDocument: doc,
--- a/toolkit/components/passwordmgr/test/test_formless_submit.html
+++ b/toolkit/components/passwordmgr/test/test_formless_submit.html
@@ -51,16 +51,26 @@ const TESTCASES = [
     formSubmitURL: DEFAULT_ORIGIN,
     usernameFieldValue: null,
     newPasswordFieldValue: "pass1",
     oldPasswordFieldValue: null,
   },
   {
     document: `<input value="user1">
       <input type=password value="pass1">`,
+    inputIndexForFormLike: 0,
+    hostname: DEFAULT_ORIGIN,
+    formSubmitURL: DEFAULT_ORIGIN,
+    usernameFieldValue: "user1",
+    newPasswordFieldValue: "pass1",
+    oldPasswordFieldValue: null,
+  },
+  {
+    document: `<input value="user1">
+      <input type=password value="pass1">`,
     inputIndexForFormLike: 1,
     hostname: DEFAULT_ORIGIN,
     formSubmitURL: DEFAULT_ORIGIN,
     usernameFieldValue: "user1",
     newPasswordFieldValue: "pass1",
     oldPasswordFieldValue: null,
   },
   {
@@ -131,17 +141,17 @@ let runTest = Task.async(function*() {
   let loginFrame = document.getElementById("loginFrame");
   let frameDoc = loginFrame.contentWindow.document;
 
   for (let tc of TESTCASES) {
     info("Starting testcase: " + JSON.stringify(tc));
     frameDoc.documentElement.innerHTML = tc.document;
     let inputForFormLike = frameDoc.querySelectorAll("input")[tc.inputIndexForFormLike];
 
-    let formLike = FormLikeFactory.createFromPasswordField(inputForFormLike);
+    let formLike = FormLikeFactory.createFromField(inputForFormLike);
 
     info("Calling _onFormSubmit with FormLike");
     let processedPromise = getSubmitMessage();
     LoginManagerContent._onFormSubmit(formLike);
 
     let submittedResult = yield processedPromise;
 
     // Check data sent via RemoteLogins:onFormSubmit
--- a/toolkit/components/passwordmgr/test/unit/test_getFormFields.js
+++ b/toolkit/components/passwordmgr/test/unit/test_getFormFields.js
@@ -13,16 +13,22 @@ const { LoginManagerContent, FormLikeFac
 const TESTCASES = [
   {
     description: "1 password field outside of a <form>",
     document: `<input id="pw1" type=password>`,
     returnedFieldIDs: [null, "pw1", null],
     skipEmptyFields: undefined,
   },
   {
+    description: "1 text field outside of a <form> without a password field",
+    document: `<input id="un1">`,
+    returnedFieldIDs: [null, null, null],
+    skipEmptyFields: undefined,
+  },
+  {
     description: "1 username & password field outside of a <form>",
     document: `<input id="un1">
       <input id="pw1" type=password>`,
     returnedFieldIDs: ["un1", "pw1", null],
     skipEmptyFields: undefined,
   },
   {
     description: "1 username & password field in a <form>",
@@ -65,16 +71,28 @@ const TESTCASES = [
   },
   {
     description: "1 password field in a form, 1 outside (not processed)",
     document: `<form><input id="pw1" type=password></form><input id="pw2" type=password>`,
     returnedFieldIDs: [null, "pw1", null],
     skipEmptyFields: undefined,
   },
   {
+    description: "1 password field in a form, 1 text field outside (not processed)",
+    document: `<form><input id="pw1" type=password></form><input>`,
+    returnedFieldIDs: [null, "pw1", null],
+    skipEmptyFields: undefined,
+  },
+  {
+    description: "1 text field in a form, 1 password field outside (not processed)",
+    document: `<form><input></form><input id="pw1" type=password>`,
+    returnedFieldIDs: [null, null, null],
+    skipEmptyFields: undefined,
+  },
+  {
     description: "2 password fields outside of a <form> with 1 linked via @form",
     document: `<input id="pw1" type=password><input id="pw2" type=password form='form1'>
       <form id="form1"></form>`,
     returnedFieldIDs: [null, "pw1", null],
     skipEmptyFields: undefined,
   },
   {
     description: "2 password fields outside of a <form> with 1 linked via @form + skipEmpty",
@@ -97,20 +115,20 @@ for (let tc of TESTCASES) {
 
   (function() {
     let testcase = tc;
     add_task(function*() {
       do_print("Starting testcase: " + testcase.description);
       let document = MockDocument.createTestDocument("http://localhost:8080/test/",
                                                       testcase.document);
 
-      let input = document.querySelector("input[type='password']");
+      let input = document.querySelector("input");
       MockDocument.mockOwnerDocumentProperty(input, document, "http://localhost:8080/test/");
 
-      let formLike = FormLikeFactory.createFromPasswordField(input);
+      let formLike = FormLikeFactory.createFromField(input);
 
       let actual = LoginManagerContent._getFormFields(formLike,
                                                       testcase.skipEmptyFields,
                                                       new Set());
 
       Assert.strictEqual(testcase.returnedFieldIDs.length, 3,
                          "_getFormFields returns 3 elements");
 
--- a/toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
+++ b/toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
@@ -11,17 +11,18 @@ const TESTCASES = [
     description: "Empty document",
     document: ``,
     returnedFieldIDsByFormLike: [],
     skipEmptyFields: undefined,
   },
   {
     description: "Non-password input with no <form> present",
     document: `<input>`,
-    returnedFieldIDsByFormLike: [],
+    // Only the IDs of password fields should be in this array
+    returnedFieldIDsByFormLike: [[]],
     skipEmptyFields: undefined,
   },
   {
     description: "1 password field outside of a <form>",
     document: `<input id="pw1" type=password>`,
     returnedFieldIDsByFormLike: [["pw1"]],
     skipEmptyFields: undefined,
   },
@@ -91,21 +92,17 @@ for (let tc of TESTCASES) {
     let testcase = tc;
     add_task(function*() {
       do_print("Starting testcase: " + testcase.description);
       let document = MockDocument.createTestDocument("http://localhost:8080/test/",
                                                      testcase.document);
 
       let mapRootElementToFormLike = new Map();
       for (let input of document.querySelectorAll("input")) {
-        if (input.type != "password") {
-          continue;
-        }
-
-        let formLike = FormLikeFactory.createFromPasswordField(input);
+        let formLike = FormLikeFactory.createFromField(input);
         let existingFormLike = mapRootElementToFormLike.get(formLike.rootElement);
         if (!existingFormLike) {
           mapRootElementToFormLike.set(formLike.rootElement, formLike);
           continue;
         }
 
         // If the formLike is already present, ensure that the properties are the same.
         do_print("Checking if the new FormLike for the same root has the same properties");