Bug 1724136 - P1. Support filtering out username-only forms by site recipes r=sfoster,tgiles
authorDimi Lee <dlee@mozilla.com>
Mon, 13 Sep 2021 12:24:49 +0000
changeset 591754 a8ca5104f6f8579b3d007ddd65391a7c2cfa9036
parent 591753 1819466dd62bef1a59ea54691c4276c64a37ea8e
child 591755 3a0f70175d72b95cd15f26207a7f0f3e6fe456a6
push id149609
push userdlee@mozilla.com
push dateMon, 13 Sep 2021 12:28:04 +0000
treeherderautoland@3a0f70175d72 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster, tgiles
bugs1724136
milestone94.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 1724136 - P1. Support filtering out username-only forms by site recipes r=sfoster,tgiles Differential Revision: https://phabricator.services.mozilla.com/D124339
toolkit/components/passwordmgr/LoginManagerChild.jsm
toolkit/components/passwordmgr/test/mochitest/test_autofill_username-only.html
toolkit/components/passwordmgr/test/unit/test_getUsernameFieldFromUsernameOnlyForm.js
--- a/toolkit/components/passwordmgr/LoginManagerChild.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerChild.jsm
@@ -1025,17 +1025,21 @@ this.LoginManagerChild = class LoginMana
     let form = event.target;
     let formLike = LoginFormFactory.createFromForm(form);
     log("_processDOMFormHasPossibleUsernameEvent:", form, formLike);
 
     // If the form contains a passoword field, `getUsernameFieldFromUsernameOnlyForm` returns
     // null, so we don't trigger autofill for those forms here. In this function,
     // we only care about username-only forms. For forms contain a password, they'll be handled
     // in onDOMFormHasPassword.
-    let usernameField = this.getUsernameFieldFromUsernameOnlyForm(form);
+
+    // We specifically set the recipe to empty here to avoid loading site recipes during page loads.
+    // This is okay because if we end up finding a username-only form that should be ignore by
+    // the site recipe, the form will be skipped while autofilling later.
+    let usernameField = this.getUsernameFieldFromUsernameOnlyForm(form, {});
     if (usernameField) {
       // Autofill the username-only form.
       log(
         "_processDOMFormHasPossibleUsernameEvent: A username-only form is found"
       );
       this._fetchLoginsFromParentAndFillForm(formLike);
     }
 
@@ -1634,18 +1638,20 @@ this.LoginManagerChild = class LoginMana
     // a password field. Note that recipes are not supported in username-only
     // forms currently (Bug 1708455).
     if (!pwFields) {
       if (!LoginHelper.usernameOnlyFormEnabled) {
         return emptyResult;
       }
 
       usernameField = this.getUsernameFieldFromUsernameOnlyForm(
-        form.rootElement
+        form.rootElement,
+        fieldOverrideRecipe
       );
+
       if (usernameField) {
         let acFieldName = usernameField.getAutocompleteInfo().fieldName;
         log(
           "Username field ",
           usernameField,
           "has name/value/autocomplete:",
           usernameField.name,
           "/",
@@ -3048,20 +3054,22 @@ this.LoginManagerChild = class LoginMana
    * following conditions:
    * 1. Does not have any password field,
    * 2. Only contains one input field whose type is username compatible.
    * 3. The username compatible input field looks like a username field
    *    or the form itself looks like a sign-in or sign-up form.
    *
    * @param {Element} formElement
    *                  the form to check.
+   * @param {Object}  recipe=null
+   *                  A relevant field override recipe to use.
    * @returns {Element} The username field or null (if the form is not a
    *                    username-only form).
    */
-  getUsernameFieldFromUsernameOnlyForm(formElement) {
+  getUsernameFieldFromUsernameOnlyForm(formElement, recipe = null) {
     if (ChromeUtils.getClassName(formElement) !== "HTMLFormElement") {
       return null;
     }
 
     let candidate = null;
     for (let element of formElement.elements) {
       // Only care input fields in the form.
       if (ChromeUtils.getClassName(element) !== "HTMLInputElement") {
@@ -3074,16 +3082,23 @@ this.LoginManagerChild = class LoginMana
         return null;
       }
 
       // Ignore input fields whose type are not username compatiable, ex, hidden.
       if (!LoginHelper.isUsernameFieldType(element)) {
         continue;
       }
 
+      if (
+        recipe?.notUsernameSelector &&
+        element.matches(recipe.notUsernameSelector)
+      ) {
+        continue;
+      }
+
       // If there are more than two input fields whose type is username
       // compatiable, this is NOT a username-only form.
       if (candidate) {
         return null;
       }
       candidate = element;
     }
 
--- a/toolkit/components/passwordmgr/test/mochitest/test_autofill_username-only.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_autofill_username-only.html
@@ -18,16 +18,23 @@ addLoginsInParent(
   [DEFAULT_ORIGIN, "https://autofill", null, "user1", "pass1"]);
 
 add_task(async function setup() {
   ok(readyPromise, "check promise is available");
   await readyPromise;
 });
 
 add_task(async function test_autofill_username_only_form() {
+  await loadRecipes({
+    siteRecipes: [{
+      hosts: ["mochi.test:8888"],
+      notUsernameSelector: "input[name='shouldnotfill']",
+    }],
+  });
+
   let win = window.open("about:blank");
   SimpleTest.registerCleanupFunction(() => win.close());
   await loadFormIntoWindow(DEFAULT_ORIGIN, `
     <!-- no password field, 1 username field -->
     <form id='form1' action='https://autofill'> 1
         <input type='text' name='uname' autocomplete='username' value=''>
 
         <button type='submit'>Submit</button>
@@ -68,24 +75,35 @@ add_task(async function test_autofill_us
     </form>
 
     <!-- no password field, 1 text input field (not a username-only form), should be ignored -->
     <form id='form6' action='https://autofill'> 6
         <input type='text' name='uname' value=''>
 
         <button type='submit'>Submit</button>
         <button type='reset'> Reset </button>
+    </form>
+
+    <!-- no password field, 1 username field that matches notUsernameSelector recipe -->
+    <form id='form7' action='https://autofill'> 7
+        <input type='text' name='shouldnotfill' autocomplete='username' value=''>
+
+        <button type='submit'>Submit</button>
+        <button type='reset'> Reset </button>
     </form>`, win);
 
   await checkLoginFormInFrameWithElementValues(win, 1, "user1");
   await checkLoginFormInFrameWithElementValues(win, 2, "someuser");
   await checkUnmodifiedFormInFrame(win, 3);
   await checkUnmodifiedFormInFrame(win, 4);
   await checkUnmodifiedFormInFrame(win, 5);
   await checkUnmodifiedFormInFrame(win, 6);
+  await checkUnmodifiedFormInFrame(win, 7);
+
+  await resetRecipes();
 });
 </script>
 
 <p id="display"></p>
 <div id="content"></div>
 <pre id="test"></pre>
 </pre>
 </body>
--- a/toolkit/components/passwordmgr/test/unit/test_getUsernameFieldFromUsernameOnlyForm.js
+++ b/toolkit/components/passwordmgr/test/unit/test_getUsernameFieldFromUsernameOnlyForm.js
@@ -3,16 +3,18 @@
  */
 
 "use strict";
 
 const { LoginManagerChild } = ChromeUtils.import(
   "resource://gre/modules/LoginManagerChild.jsm"
 );
 
+// expectation[0] tests cases when a form doesn't have a sign-in keyword.
+// expectation[1] tests cases when a form has a sign-in keyword.
 const TESTCASES = [
   {
     description: "1 text input field",
     document: `<form>
       <input id="un1" type="text">
       </form>`,
     expectations: [false, true],
   },
@@ -110,16 +112,27 @@ const TESTCASES = [
   },
   {
     description: "Form with only a button",
     document: `<form>
       <input id="un1" type="button" autocomplete="username">
       </form>`,
     expectations: [false, false],
   },
+  {
+    description: "A username only form matches not username selector",
+    document: `<form>
+      <input id="un1" type="text" name="secret_username">
+      </form>`,
+    fieldOverrideRecipe: {
+      hosts: ["localhost:8080"],
+      notUsernameSelector: 'input[name="secret_username"]',
+    },
+    expectations: [false, false],
+  },
 ];
 
 function _setPrefs() {
   Services.prefs.setBoolPref("signon.usernameOnlyForm.enabled", true);
   registerCleanupFunction(() => {
     Services.prefs.clearUserPref("signon.usernameOnlyForm.enabled");
   });
 }
@@ -145,17 +158,20 @@ for (let tc of TESTCASES) {
         );
 
         let form = document.querySelector("form");
         if (formHasSigninKeyword) {
           form.setAttribute("name", "login");
         }
 
         let lmc = new LoginManagerChild();
-        let element = lmc.getUsernameFieldFromUsernameOnlyForm(form);
+        let element = lmc.getUsernameFieldFromUsernameOnlyForm(
+          form,
+          testcase.fieldOverrideRecipe
+        );
         Assert.strictEqual(
           testcase.expectations[formHasSigninKeyword ? 1 : 0],
           element != null,
           `Return incorrect result when the layout is ${testcase.description}`
         );
       });
     })();
   }