Bug 1119063 - Don't autofill password fields with autocomplete=new-password. r=MattN
authorSam Foster <sfoster@mozilla.com>
Thu, 28 Feb 2019 21:24:33 +0000
changeset 519704 0cb8b8db9618f7e453be5f1b4c12a01a348ed082
parent 519703 e8b9a7f1d2097115239a287b9a1f93d9fef374b4
child 519705 f6599af67fa8848d2e653b80f2d4894c8382afed
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1119063
milestone67.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 1119063 - Don't autofill password fields with autocomplete=new-password. r=MattN * Add form autofill outcome AUTOFILL_RESULT.PASSWORD_AUTOCOMPLETE_NEW_PASSWORD * Autocomplete behavior is not changed Differential Revision: https://phabricator.services.mozilla.com/D21274
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/mochitest/mochitest.ini
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_new_password.html
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1101,16 +1101,17 @@ var LoginManagerContent = {
    *        If this is false and no inputElement is passed, if the username
    *        field value is not found in foundLogins, it will not fill the
    *        password.
    * @param {bool} [options.clobberPassword = false] controls if an existing
    *        password value can be overwritten
    * @param {bool} [options.userTriggered = false] an indication of whether
    *        this filling was triggered by the user
    */
+  // eslint-disable-next-line complexity
   _fillForm(form, foundLogins, recipes, {
     inputElement = null,
     autofillForm = false,
     clobberUsername = false,
     clobberPassword = false,
     userTriggered = false,
   } = {}) {
     if (ChromeUtils.getClassName(form) === "HTMLFormElement") {
@@ -1128,16 +1129,17 @@ var LoginManagerContent = {
       NO_LOGINS_FIT: 3,
       NO_SAVED_LOGINS: 4,
       EXISTING_PASSWORD: 5,
       EXISTING_USERNAME: 6,
       MULTIPLE_LOGINS: 7,
       NO_AUTOFILL_FORMS: 8,
       AUTOCOMPLETE_OFF: 9,
       INSECURE: 10,
+      PASSWORD_AUTOCOMPLETE_NEW_PASSWORD: 11,
     };
 
     try {
       // Nothing to do if we have no matching logins available,
       // and there isn't a need to show the insecure form warning.
       if (foundLogins.length == 0 &&
           (InsecurePasswordUtils.isFormSecure(form) ||
           !LoginHelper.showInsecureFieldWarning)) {
@@ -1242,16 +1244,24 @@ var LoginManagerContent = {
       }, this);
 
       if (logins.length == 0) {
         log("form not filled, none of the logins fit in the field");
         autofillResult = AUTOFILL_RESULT.NO_LOGINS_FIT;
         return;
       }
 
+      // If the password field has the autocomplete value of "new-password"
+      // and we're autofilling without user interaction, there's nothing to do.
+      if (!userTriggered && passwordField.getAutocompleteInfo().fieldName == "new-password") {
+        log("not filling form, password field has the autocomplete new-password value");
+        autofillResult = AUTOFILL_RESULT.PASSWORD_AUTOCOMPLETE_NEW_PASSWORD;
+        return;
+      }
+
       // Don't clobber an existing password.
       if (passwordField.value && !clobberPassword) {
         log("form not filled, the password field was already filled");
         autofillResult = AUTOFILL_RESULT.EXISTING_PASSWORD;
         return;
       }
 
       // Select a login to use for filling in the form.
--- a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
+++ b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
@@ -16,16 +16,18 @@ support-files =
   ../browser/formless_basic.html
   ../browser/form_cross_origin_secure_action.html
   auth2/authenticate.sjs
   pwmgr_common.js
   pwmgr_common_parent.js
   ../authenticate.sjs
 skip-if = toolkit == 'android' && !is_fennec # Don't run on GeckoView
 
+# Note: new tests should use scheme = https unless they have a specific reason not to
+
 [test_autocomplete_highlight.html]
 scheme = https
 skip-if = toolkit == 'android' # autocomplete
 [test_autocomplete_https_upgrade.html]
 skip-if = toolkit == 'android' # autocomplete
 [test_autocomplete_sandboxed.html]
 scheme = https
 skip-if = toolkit == 'android' # autocomplete
@@ -77,16 +79,19 @@ skip-if = toolkit == 'android' && debug 
 [test_input_events_for_identical_values.html]
 [test_master_password.html]
 scheme = https
 skip-if = os != 'mac' # Tests desktop prompts and bug 1333264
 support-files =
   chrome_timeout.js
   subtst_master_pass.html
 [test_maxlength.html]
+[test_autocomplete_new_password.html]
+scheme = https
+skip-if = toolkit == 'android' # autocomplete
 [test_onsubmit_value_change.html]
 [test_passwords_in_type_password.html]
 [test_prompt.html]
 skip-if = os == "linux" || toolkit == 'android' # Tests desktop prompts
 [test_prompt_async.html]
 skip-if = toolkit == 'android' # Tests desktop prompts
 support-files = subtst_prompt_async.html
 [test_prompt_http.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/mochitest/test_autocomplete_new_password.html
@@ -0,0 +1,107 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test autofill with autocomplete=new-password fields</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="text/javascript" src="pwmgr_common.js"></script>
+  <script type="text/javascript" src="../../../satchel/test/satchel_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+Login Manager test: autofill with autocomplete=new-password fields
+
+<script>
+let chromeScript = runInParent(function initLogins() {
+  const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+  let login1 = Cc["@mozilla.org/login-manager/loginInfo;1"]
+                        .createInstance(Ci.nsILoginInfo);
+  login1.init("https://example.com", "https://autofill", null,
+              "user1", "pass1", "", "");
+
+  Services.logins.addLogin(login1);
+});
+
+let readyPromise = registerRunTests();
+</script>
+<p id="display"></p>
+
+<!-- we presumably can't hide the content for this test. -->
+<div id="content">
+
+  <!-- form1 is the reference, sanity-check -->
+  <form id="form1" action="https://autofill" onsubmit="return false;">
+    <input type="text" name="uname">
+    <input type="password" name="p">
+    <button type="submit">Submit</button>
+  </form>
+
+  <!-- form2 uses the new-password field -->
+  <form id="form2" action="https://autofill" onsubmit="return false;">
+    <input type="text" name="uname">
+    <input type="password" autocomplete="new-password">
+    <button type="submit">Submit</button>
+  </form>
+</div>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+const {ContentTaskUtils} =
+  SpecialPowers.Cu.import("resource://testing-common/ContentTaskUtils.jsm", {});
+
+async function getAutocompleteResult(input, expectedValues) {
+  input.focus();
+
+  const shownPromise = promiseACShown();
+  synthesizeKey("KEY_ArrowDown");
+  await shownPromise;
+  synthesizeKey("KEY_ArrowDown");
+  await synthesizeKey("KEY_Enter");
+
+  let didAutocomplete;
+  try {
+    await ContentTaskUtils.waitForCondition(() => {
+      for (let [selector, expectedValue] of Object.entries(expectedValues)) {
+        if (document.querySelector(selector).value !== expectedValue) {
+          return false;
+        }
+      }
+      return true;
+    });
+    didAutocomplete = true;
+  } catch (ex) {
+    info("waitForCondition exception: " + ex.message);
+    didAutocomplete = false;
+  }
+  return didAutocomplete;
+}
+
+add_task(async function setup() {
+  ok(readyPromise, "check promise is available");
+  await readyPromise;
+});
+
+add_task(async function test_autofillAutocompleteNewPassword() {
+  // reference form was filled as expected?
+  checkForm(1, "user1", "pass1");
+
+  // 2nd form should not be filled
+  checkForm(2, "", "");
+
+  let form = document.getElementById("form2");
+  let userInput = form.querySelector("[name='uname']");
+
+  const didAutocomplete = await getAutocompleteResult(userInput, {
+    "#form2 [name='uname']": "user1",
+    "#form2 [type='password']": "pass1",
+  });
+  ok(didAutocomplete, "Autocomplete of user and password fields should happen");
+});
+
+</script>
+</pre>
+</body>
+</html>