Bug 1192081 - Changed createFromPasswordField to also create formlikes from text fields. r=MattN draft
authorBernardo P. Rittmeyer <bernardo@rittme.com>
Mon, 10 Aug 2015 17:33:13 -0700
changeset 284254 3a0a5a863d516e372eae74be86ad41b9e7437a5e
parent 284166 7a19194812eb767bee7cdf8fc36ba9a383c1bead
child 508209 ce11638ed9a7c89cdd00ee27f4e94ccf515b402d
push id4256
push userbernardo@rittme.com
push dateTue, 11 Aug 2015 00:33:48 +0000
reviewersMattN
bugs1192081
milestone42.0a1
Bug 1192081 - Changed createFromPasswordField to also create formlikes from text fields. r=MattN
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/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.createFromInputField(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.createFromInputField(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);
@@ -606,29 +606,33 @@ var LoginManagerContent = {
    * @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 than we can create a formLike from a text field,
+   * this method will only return a non-null usernameField if the
+   * formLike have 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.createFromInputField(pwOverrideField);
         pwFields = [{
           index   : [...formLike.elements].indexOf(pwOverrideField),
           element : pwOverrideField,
         }];
       }
 
       var usernameOverrideField = LoginRecipesContent.queryLoginField(
         form,
@@ -1219,32 +1223,32 @@ let FormLikeFactory = {
    * Create a FormLike object from an <input type=password>.
    *
    * If the <input> 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} aInputField - a password field in a document
    * @return {FormLike}
-   * @throws Error if aPasswordField isn't a password input in a document
+   * @throws Error if aInputField isn't a password input 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");
+  createFromInputField(aInputField) {
+    if (!(aInputField instanceof Ci.nsIDOMHTMLInputElement) ||
+        (aInputField.type != "password" && aInputField.type != "text") ||
+        !aInputField.ownerDocument) {
+      throw new Error("createFromInputField requires an input field in a document");
     }
 
-    if (aPasswordField.form) {
-      return this.createFromForm(aPasswordField.form);
+    if (aInputField.form) {
+      return this.createFromForm(aInputField.form);
     }
 
-    let doc = aPasswordField.ownerDocument;
+    let doc = aInputField.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.createFromInputField(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>",
+    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.createFromInputField(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 at 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.createFromInputField(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");