Bug 1330829 - Login Manager recipe support for ignoring specific password fields. r=johannh
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Fri, 13 Jan 2017 13:34:55 -0800
changeset 374421 fc4d5c2cade3b7822267b06c6554f3335b2f3c45
parent 374420 68c824ad72b817189143af771bb34408c72ad495
child 374422 5fd77a2de293546a22f6f7dccf2fc733e1f29272
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1330829
milestone53.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 1330829 - Login Manager recipe support for ignoring specific password fields. r=johannh MozReview-Commit-ID: 5Y2pTaPDGb5
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/LoginRecipes.jsm
toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html
toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -580,33 +580,46 @@ var LoginManagerContent = {
           .then(null, Cu.reportError);
     } else {
       // Ignore the event, it's for some input we don't care about.
     }
   },
 
   /**
    * @param {FormLike} form - the FormLike to look for password fields in.
-   * @param {bool} [skipEmptyFields=false] - Whether to ignore password fields with no value.
-   *                                         Used at capture time since saving empty values isn't
-   *                                         useful.
+   * @param {Object} options
+   * @param {bool} [options.skipEmptyFields=false] - Whether to ignore password fields with no value.
+   *                                                 Used at capture time since saving empty values isn't
+   *                                                 useful.
+   * @param {Object} [options.fieldOverrideRecipe=null] - A relevant field override recipe to use.
    * @return {Array|null} Array of password field elements for the specified form.
    *                      If no pw fields are found, or if more than 3 are found, then null
    *                      is returned.
    */
-  _getPasswordFields(form, skipEmptyFields = false) {
+  _getPasswordFields(form, {
+    fieldOverrideRecipe = null,
+    skipEmptyFields = false,
+  } = {}) {
     // Locate the password fields in the form.
     let pwFields = [];
     for (let i = 0; i < form.elements.length; i++) {
       let element = form.elements[i];
       if (!(element instanceof Ci.nsIDOMHTMLInputElement) ||
           element.type != "password") {
         continue;
       }
 
+      // Exclude ones matching a `notPasswordSelector`, if specified.
+      if (fieldOverrideRecipe && fieldOverrideRecipe.notPasswordSelector &&
+          element.matches(fieldOverrideRecipe.notPasswordSelector)) {
+        log("skipping password field (id/name is", element.id, " / ",
+            element.name + ") due to recipe:", fieldOverrideRecipe);
+        continue;
+      }
+
       if (skipEmptyFields && !element.value) {
         continue;
       }
 
       pwFields[pwFields.length] = {
                                     index   : i,
                                     element
                                   };
@@ -670,17 +683,20 @@ var LoginManagerContent = {
       if (usernameOverrideField) {
         usernameField = usernameOverrideField;
       }
     }
 
     if (!pwFields) {
       // Locate the password field(s) in the form. Up to 3 supported.
       // If there's no password field, there's nothing for us to do.
-      pwFields = this._getPasswordFields(form, isSubmission);
+      pwFields = this._getPasswordFields(form, {
+        fieldOverrideRecipe,
+        skipEmptyFields: isSubmission,
+      });
     }
 
     if (!pwFields) {
       return [null, null, null];
     }
 
     if (!usernameField) {
       // Locate the username field in the form by searching backwards
--- a/toolkit/components/passwordmgr/LoginRecipes.jsm
+++ b/toolkit/components/passwordmgr/LoginRecipes.jsm
@@ -3,17 +3,24 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["LoginRecipesContent", "LoginRecipesParent"];
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 const REQUIRED_KEYS = ["hosts"];
-const OPTIONAL_KEYS = ["description", "notUsernameSelector", "passwordSelector", "pathRegex", "usernameSelector"];
+const OPTIONAL_KEYS = [
+  "description",
+  "notPasswordSelector",
+  "notUsernameSelector",
+  "passwordSelector",
+  "pathRegex",
+  "usernameSelector",
+];
 const SUPPORTED_KEYS = REQUIRED_KEYS.concat(OPTIONAL_KEYS);
 
 Cu.importGlobalProperties(["URL"]);
 
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
@@ -221,17 +228,17 @@ var LoginRecipesContent = {
     if (!recipes.size) {
       return null;
     }
 
     let chosenRecipe = null;
     // Find the first (most-specific recipe that involves field overrides).
     for (let recipe of recipes) {
       if (!recipe.usernameSelector && !recipe.passwordSelector &&
-          !recipe.notUsernameSelector) {
+          !recipe.notUsernameSelector && !recipe.notPasswordSelector) {
         continue;
       }
 
       chosenRecipe = recipe;
       break;
     }
 
     return chosenRecipe;
--- a/toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html
@@ -128,16 +128,80 @@ add_task(function* testNotUsernameFieldN
     </form>`;
 
   let elements = yield waitForFills(1);
   for (let element of elements) {
     is(element.dataset.expected, "true", `${element.name} was filled`);
   }
 });
 
+add_task(function* loadNotPasswordSelectorRecipes() {
+  yield resetRecipes();
+  yield loadRecipes({
+    siteRecipes: [{
+      hosts: ["mochi.test:8888"],
+      notPasswordSelector: "input[name='not_pword'], input[name='not_pword2']",
+    }],
+  });
+});
+
+add_task(function* testNotPasswordField() {
+  document.getElementById("content").innerHTML = `
+    <!-- The field matching notPasswordSelector should be skipped -->
+    <form id="form5">
+      <input type="text"     name="uname7" data-expected="true">
+      <input type="password" name="not_pword" data-expected="false">
+      <input type="password" name="pword7" data-expected="true">
+    </form>`;
+
+  let elements = yield waitForFills(2);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
+});
+
+add_task(function* testNotPasswordFieldNoPassword() {
+  document.getElementById("content").innerHTML = `
+    <!-- The field matching notPasswordSelector should be skipped.
+         No username or password field should be found and filled in this case.
+         A dummy form7 is added after so we know when the login manager is done
+         considering filling form6. -->
+    <form id="form6">
+      <input type="text"     name="uname8" data-expected="false">
+      <input type="password" name="not_pword" data-expected="false">
+    </form>
+    <form id="form7">
+      <input type="password" name="pword9" data-expected="true">
+    </form>`;
+
+  let elements = yield waitForFills(1);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
+});
+
+add_task(function* testNotPasswordField_tooManyToOkay() {
+  document.getElementById("content").innerHTML = `
+    <!-- The field matching notPasswordSelector should be skipped so we won't
+         have too many pw fields to handle (3). -->
+    <form id="form8">
+      <input type="text"     name="uname9" data-expected="true">
+      <input type="password" name="not_pword2" data-expected="false">
+      <input type="password" name="not_pword" data-expected="false">
+      <input type="password" name="pword10" data-expected="true">
+      <input type="password" name="pword11" data-expected="false">
+      <input type="password" name="pword12" data-expected="false">
+    </form>`;
+
+  let elements = yield waitForFills(2);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
+});
+
 </script>
 
 <p id="display"></p>
 
 <div id="content">
   // Forms are inserted dynamically
 </div>
 <pre id="test"></pre>
--- a/toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
+++ b/toolkit/components/passwordmgr/test/unit/test_getPasswordFields.js
@@ -77,16 +77,31 @@ const TESTCASES = [
     skipEmptyFields: true,
   },
   {
     description: "2 password fields outside of a <form> with 1 linked via @form + skipEmpty with 1 empty",
     document: `<input id="pw1" type=password value="pass1"><input id="pw2" type=password form="form1">
       <form id="form1"></form>`,
     returnedFieldIDsByFormLike: [["pw1"], []],
     skipEmptyFields: true,
+    fieldOverrideRecipe: {
+      // Ensure a recipe without `notPasswordSelector` doesn't cause a problem.
+      hosts: ["localhost:8080"],
+    },
+  },
+  {
+    description: "3 password fields outside of a <form> with 1 linked via @form + skipEmpty",
+    document: `<input id="pw1" type=password value="pass1"><input id="pw2" type=password form="form1" value="pass2"><input id="pw3" type=password value="pass3">
+      <form id="form1"><input id="pw4" type=password></form>`,
+    returnedFieldIDsByFormLike: [["pw3"], ["pw2"]],
+    skipEmptyFields: true,
+    fieldOverrideRecipe: {
+      hosts: ["localhost:8080"],
+      notPasswordSelector: "#pw1",
+    },
   },
 ];
 
 for (let tc of TESTCASES) {
   do_print("Sanity checking the testcase: " + tc.description);
 
   (function() {
     let testcase = tc;
@@ -110,18 +125,20 @@ for (let tc of TESTCASES) {
       }
 
       Assert.strictEqual(mapRootElementToFormLike.size, testcase.returnedFieldIDsByFormLike.length,
                          "Check the correct number of different formLikes were returned");
 
       let formLikeIndex = -1;
       for (let formLikeFromInput of mapRootElementToFormLike.values()) {
         formLikeIndex++;
-        let pwFields = LoginManagerContent._getPasswordFields(formLikeFromInput,
-                                                              testcase.skipEmptyFields);
+        let pwFields = LoginManagerContent._getPasswordFields(formLikeFromInput, {
+          fieldOverrideRecipe: testcase.fieldOverrideRecipe,
+          skipEmptyFields: testcase.skipEmptyFields,
+        });
 
         if (formLikeFromInput.rootElement instanceof Ci.nsIDOMHTMLFormElement) {
           let formLikeFromForm = LoginFormFactory.createFromForm(formLikeFromInput.rootElement);
           do_print("Checking that the FormLike created for the <form> matches" +
                    " the one from a password field");
           formLikeEqual(formLikeFromInput, formLikeFromForm);
         }