Bug 1531135 - Honor autocomplete=off on password when signon.autofillForms.autocompleteOff is false. r=MattN
authorSam Foster <sfoster@mozilla.com>
Thu, 07 Mar 2019 01:12:11 +0000
changeset 520846 6261644bec1744c7df50c8b775e9a173eb8982ff
parent 520845 4dc7ad52faf227101331c22f1ce4babb475661f0
child 520847 7450a570330eb18ff2bd23f129e44566c964fb0b
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
bugs1531135
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 1531135 - Honor autocomplete=off on password when signon.autofillForms.autocompleteOff is false. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D22025
modules/libpref/init/all.js
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/mochitest/mochitest.ini
toolkit/components/passwordmgr/test/mochitest/test_basic_form_honor_autocomplete_off.html
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4670,16 +4670,17 @@ pref("font.name-list.monospace.x-unicode
 
 # AIX
 #endif
 
 // Login Manager prefs
 pref("signon.rememberSignons",              true);
 pref("signon.rememberSignons.visibilityToggle", true);
 pref("signon.autofillForms",                true);
+pref("signon.autofillForms.autocompleteOff", true);
 pref("signon.autofillForms.http",           false);
 pref("signon.autologin.proxy",              false);
 pref("signon.formlessCapture.enabled",      true);
 pref("signon.privateBrowsingCapture.enabled", false);
 pref("signon.storeWhenAutocompleteOff",     true);
 pref("signon.debug",                        false);
 pref("signon.recipes.path",                 "chrome://passwordmgr/content/recipes.json");
 pref("signon.schemeUpgrades",               false);
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -35,16 +35,17 @@ var LoginHelper = {
   init() {
     // Watch for pref changes to update cached pref values.
     Services.prefs.addObserver("signon.", () => this.updateSignonPrefs());
     this.updateSignonPrefs();
   },
 
   updateSignonPrefs() {
     this.autofillForms = Services.prefs.getBoolPref("signon.autofillForms");
+    this.autofillAutocompleteOff = Services.prefs.getBoolPref("signon.autofillForms.autocompleteOff");
     this.debug = Services.prefs.getBoolPref("signon.debug");
     this.enabled = Services.prefs.getBoolPref("signon.rememberSignons");
     this.formlessCaptureEnabled = Services.prefs.getBoolPref("signon.formlessCapture.enabled");
     this.insecureAutofill = Services.prefs.getBoolPref("signon.autofillForms.http");
     this.privateBrowsingCaptureEnabled =
       Services.prefs.getBoolPref("signon.privateBrowsingCapture.enabled");
 
     this.schemeUpgrades = Services.prefs.getBoolPref("signon.schemeUpgrades");
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1117,17 +1117,16 @@ var LoginManagerContent = {
     clobberPassword = false,
     userTriggered = false,
   } = {}) {
     if (ChromeUtils.getClassName(form) === "HTMLFormElement") {
       throw new Error("_fillForm should only be called with FormLike objects");
     }
 
     log("_fillForm", form.elements);
-    let ignoreAutocomplete = true;
     // Will be set to one of AUTOFILL_RESULT in the `try` block.
     let autofillResult = -1;
     const AUTOFILL_RESULT = {
       FILLED: 0,
       NO_PASSWORD_FIELD: 1,
       PASSWORD_DISABLED_READONLY: 2,
       NO_LOGINS_FIT: 3,
       NO_SAVED_LOGINS: 4,
@@ -1209,23 +1208,16 @@ var LoginManagerContent = {
       // Prevent autofilling insecure forms.
       if (!userTriggered && !LoginHelper.insecureAutofill &&
           !InsecurePasswordUtils.isFormSecure(form)) {
         log("not filling form since it's insecure");
         autofillResult = AUTOFILL_RESULT.INSECURE;
         return;
       }
 
-      var isAutocompleteOff = false;
-      if (this._isAutocompleteDisabled(form) ||
-          this._isAutocompleteDisabled(usernameField) ||
-          this._isAutocompleteDisabled(passwordField)) {
-        isAutocompleteOff = true;
-      }
-
       // Discard logins which have username/password values that don't
       // fit into the fields (as specified by the maxlength attribute).
       // The user couldn't enter these values anyway, and it helps
       // with sites that have an extra PIN to be entered (bug 391514)
       var maxUsernameLen = Number.MAX_VALUE;
       var maxPasswordLen = Number.MAX_VALUE;
 
       // If attribute wasn't set, default is -1.
@@ -1247,19 +1239,21 @@ 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;
       }
 
+      const passwordACFieldName = passwordField.getAutocompleteInfo().fieldName;
+
       // 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") {
+      if (!userTriggered && passwordACFieldName == "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");
@@ -1320,26 +1314,26 @@ var LoginManagerContent = {
       // We will always have a selectedLogin at this point.
 
       if (!autofillForm) {
         log("autofillForms=false but form can be filled");
         autofillResult = AUTOFILL_RESULT.NO_AUTOFILL_FORMS;
         return;
       }
 
-      if (isAutocompleteOff && !ignoreAutocomplete) {
-        log("Not filling the login because we're respecting autocomplete=off");
+      if (!userTriggered && passwordACFieldName == "off" && !LoginHelper.autofillAutocompleteOff) {
+        log("Not autofilling the login because we're respecting autocomplete=off");
         autofillResult = AUTOFILL_RESULT.AUTOCOMPLETE_OFF;
         return;
       }
 
       // Fill the form
 
       if (usernameField) {
-      // Don't modify the username field if it's disabled or readOnly so we preserve its case.
+        // Don't modify the username field if it's disabled or readOnly so we preserve its case.
         let disabledOrReadOnly = usernameField.disabled || usernameField.readOnly;
 
         let userNameDiffers = selectedLogin.username != usernameField.value;
         // Don't replace the username if it differs only in case, and the user triggered
         // this autocomplete. We assume that if it was user-triggered the entered text
         // is desired.
         let userEnteredDifferentCase = userTriggered && userNameDiffers &&
                usernameField.value.toLowerCase() == selectedLogin.username.toLowerCase();
--- a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
+++ b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
@@ -46,16 +46,19 @@ skip-if = toolkit == 'android' # autocom
 [test_basic_form_0pw.html]
 [test_basic_form_1pw.html]
 [test_basic_form_1pw_2.html]
 [test_basic_form_2pw_1.html]
 [test_basic_form_2pw_2.html]
 [test_basic_form_3pw_1.html]
 [test_basic_form_autocomplete.html]
 skip-if = toolkit == 'android' # android:autocomplete.
+[test_basic_form_honor_autocomplete_off.html]
+scheme = https
+skip-if = toolkit == 'android' # android:autocomplete.
 [test_insecure_form_field_autocomplete.html]
 skip-if = toolkit == 'android' || os == 'linux' # android:autocomplete., linux: bug 1325778
 [test_password_field_autocomplete.html]
 skip-if = toolkit == 'android' # android:autocomplete.
 [test_insecure_form_field_no_saved_login.html]
 skip-if = toolkit == 'android' # android:autocomplete.
 [test_basic_form_html5.html]
 [test_basic_form_pwevent.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/mochitest/test_basic_form_honor_autocomplete_off.html
@@ -0,0 +1,165 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test login autofill autocomplete when signon.autofillForms.autocompleteOff is false</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="../../../satchel/test/satchel_common.js"></script>
+  <script type="text/javascript" src="pwmgr_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+Login Manager test: autofilling when autocomplete=off
+
+<script>
+let readyPromise = registerRunTests();
+
+runInParent(function setup() {
+  const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+  // Create some logins just for this form, since we'll be deleting them.
+  const nsLoginInfo = Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
+                                             Ci.nsILoginInfo, "init");
+  assert.ok(nsLoginInfo != null, "nsLoginInfo constructor");
+
+  const login = new nsLoginInfo("https://example.com", "https://autocomplete2", null,
+                                "singleuser", "singlepass", "uname", "pword");
+  Services.logins.addLogin(login);
+});
+</script>
+<p id="display"></p>
+
+<!-- we presumably can't hide the content for this test. -->
+<div id="content">
+  <!-- test single logins, with autocomplete=off set -->
+  <form id="form1" action="https://autocomplete2" onsubmit="return false;">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword" autocomplete="off">
+    <button type="submit">Submit</button>
+  </form>
+
+  <form id="form2" action="https://autocomplete2" onsubmit="return false;">
+    <input  type="text"       name="uname" autocomplete="off">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
+
+  <form id="form3" action="https://autocomplete2" onsubmit="return false;" autocomplete="off">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
+
+  <form id="form4" action="https://autocomplete2" onsubmit="return false;">
+    <input  type="text"       name="uname" autocomplete="off">
+    <input  type="password"   name="pword" autocomplete="off">
+    <button type="submit">Submit</button>
+  </form>
+
+  <!-- control -->
+  <form id="form5" action="https://autocomplete2" onsubmit="return false;">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+/** Test for Login Manager: multiple login autocomplete. **/
+let {ContentTaskUtils} = SpecialPowers.Cu.import("resource://testing-common/ContentTaskUtils.jsm", {});
+
+// Set the pref before the document loads.
+SpecialPowers.setBoolPref("signon.autofillForms.autocompleteOff", false);
+
+SimpleTest.registerCleanupFunction(() => {
+  SpecialPowers.clearUserPref("signon.autofillForms.autocompleteOff");
+});
+
+// Check for expected username/password in form.
+function checkFormValues(form, expectedUsername, expectedPassword) {
+  let uname = form.querySelector("[name='uname']");
+  let pword = form.querySelector("[name='pword']");
+  is(uname.value, expectedUsername, `Checking ${form.id} username is: ${expectedUsername}`);
+  is(pword.value, expectedPassword, `Checking ${form.id} password is: ${expectedPassword}`);
+}
+
+async function autoCompleteFieldsFromFirstMatch(form) {
+  // trigger autocomplete from the username field
+  await SimpleTest.promiseFocus(form.ownerGlobal);
+  let uname = form.querySelector("[name='uname']");
+  let shownPromise = promiseACShown();
+  uname.focus();
+  await shownPromise;
+
+  let formFilled = promiseFormsProcessed();
+  await synthesizeKey("KEY_ArrowDown"); // open
+  await synthesizeKey("KEY_Enter");
+  await formFilled;
+  await Promise.resolve();
+}
+
+add_task(async function setup() {
+  ok(readyPromise, "check promise is available");
+  await readyPromise;
+  listenForUnexpectedPopupShown();
+});
+
+/* Tests for autofill of single-user forms for when we honor autocomplete=off on password fields */
+add_task(async function test_form1_honor_password_autocomplete_off() {
+  await SimpleTest.promiseFocus(window);
+  // With the pref toggled off, and with autocomplete=off on the password field,
+  // we expect not to have autofilled this form
+  let form = document.getElementById("form1");
+  ok(form, "found form under test");
+  checkFormValues(form, "", "");
+
+  // ..but it should autocomplete just fine
+  await autoCompleteFieldsFromFirstMatch(form);
+  checkFormValues(form, "singleuser", "singlepass");
+});
+
+add_task(async function test_form2_honor_password_autocomplete_off() {
+  await SimpleTest.promiseFocus(window);
+  // With the pref toggled off, and with autocomplete=off on the username field,
+  // we expect to have autofilled this form
+  let form = document.getElementById("form2");
+  ok(form, "found form under test");
+  checkFormValues(form, "singleuser", "singlepass");
+});
+
+add_task(async function test_form3_honor_password_autocomplete_off() {
+  await SimpleTest.promiseFocus(window);
+  // With the pref toggled off, and with autocomplete=off on the form,
+  // we expect to have autofilled this form
+  let form = document.getElementById("form3");
+  ok(form, "found form under test");
+  checkFormValues(form, "singleuser", "singlepass");
+});
+
+add_task(async function test_form4_honor_password_autocomplete_off() {
+  await SimpleTest.promiseFocus(window);
+  // With the pref toggled off, and autocomplete=off on the username and password field,
+  // we expect not to have autofilled this form
+  let form = document.getElementById("form4");
+  ok(form, "found form under test");
+  checkFormValues(form, "", "");
+
+  // ..but it should autocomplete just fine
+  await autoCompleteFieldsFromFirstMatch(form);
+  checkFormValues(form, "singleuser", "singlepass");
+});
+
+add_task(async function test_form5() {
+  await SimpleTest.promiseFocus(window);
+  // (this is a control, w/o autocomplete=off, to ensure the login
+  // that was being suppressed would have been filled in otherwise)
+  let form = document.getElementById("form5");
+  ok(form, "found form under test");
+  checkFormValues(form, "singleuser", "singlepass");
+});
+</script>
+</pre>
+</body>
+</html>