Bug 1616945 - Exclude fields with no defaultValue when evaluating if a form was modified and needs login capture. r=MattN
authorSam Foster <sfoster@mozilla.com>
Fri, 21 Feb 2020 18:09:34 +0000
changeset 515026 3d7075ed204f2328d4520934bfc63a1958340ef0
parent 515025 2f1fdc659bdbd687c51c447824b3e4e508dc13b1
child 515027 3424a68c227cd30eed34e2123934ae5fca96d7ff
push id37148
push userbtara@mozilla.com
push dateFri, 21 Feb 2020 21:49:11 +0000
treeherdermozilla-central@923415cae003 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1616945
milestone75.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 1616945 - Exclude fields with no defaultValue when evaluating if a form was modified and needs login capture. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D63577
toolkit/components/passwordmgr/LoginManagerChild.jsm
toolkit/components/passwordmgr/test/mochitest/subtst_prefilled_form.html
toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
--- a/toolkit/components/passwordmgr/LoginManagerChild.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerChild.jsm
@@ -2233,17 +2233,18 @@ this.LoginManagerChild = class LoginMana
     let state = this.stateForDocument(form.rootElement.ownerDocument);
     // check for user inputs to the form fields
     let fieldsModified = state.fieldModificationsByRootElement.get(
       form.rootElement
     );
     // also consider a form modified if there's a difference between fields' .value and .defaultValue
     if (!fieldsModified) {
       fieldsModified = Array.from(form.elements).some(
-        field => field.value !== field.defaultValue
+        field =>
+          field.defaultValue !== undefined && field.value !== field.defaultValue
       );
     }
     return fieldsModified;
   }
 
   /**
    * Given a field, determine whether that field was last filled as a username
    * field AND whether the username is still filled in with the username AND
--- a/toolkit/components/passwordmgr/test/mochitest/subtst_prefilled_form.html
+++ b/toolkit/components/passwordmgr/test/mochitest/subtst_prefilled_form.html
@@ -1,12 +1,18 @@
 <!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 
-<!-- Simplest form with username and password fields pre-populated. -->
+<!-- Form with username and password fields pre-populated.
+     With a couple elements where .defaultValue !== .value -->
 <form id="form-basic">
   <input id="form-basic-username" name="username" value="user">
   <input id="form-basic-password" name="password" type="password" value="pass">
+  <select name="picker">
+  	<option value="foo">0</option>
+  	<option value="bar" selected>1</option>
+  </select>
   <input id="form-basic-submit" type="submit">
+  <button id="form-basic-reset" type="reset">Reset</button>
 </form>
 
 </body></html>
--- a/toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
@@ -48,34 +48,16 @@ async function setup(pageUrl) {
   await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
     let doc = this.content.document;
     let link = doc.createElement("a");
     link.setAttribute("href", "http://mochi.test:8888");
     doc.body.appendChild(link);
   });
 }
 
-async function checkIframedUnmodifiedForm(formSelector = "form") {
-  let wasModified = await SpecialPowers.spawn(getIframeBrowsingContext(window), [formSelector], function(sel) {
-    let form = content.document.querySelector(sel);
-    let inputs = Array.from(form.elements || form.querySelectorAll("input"));
-    for (let ele of inputs) {
-      // No point in checking form submit/reset buttons.
-      if (ele.type == "submit" || ele.type == "reset") {
-        continue;
-      }
-      if (ele.value !== ele.defaultValue) {
-        return true;
-      }
-    }
-    return false;
-  });
-  ok(!wasModified, "Expected form to have no modified values");
-}
-
 async function clickLink() {
   let loadPromise = waitForLoad();
   await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
     let doc = this.content.document;
     doc.querySelector("a[href]").click();
   });
   await loadPromise;
 }
@@ -91,17 +73,16 @@ add_task(async function test_init() {
     ["signon.userInputRequiredToCapture.enabled", true],
   ]});
 });
 
 add_task(async function test_no_message_on_navigation() {
   // If login field values were set by the website, we don't message to save the
   // login values if the user did not interact with the fields before submiting.
   await setup(PREFILLED_FORM_URL);
-  await checkIframedUnmodifiedForm();
 
   let submitMessageSent = false;
   getSubmitMessage().then(value => {
     submitMessageSent = true;
   });
   await clickLink();
 
   // allow time to pass before concluding no onFormSubmit message was sent
@@ -110,40 +91,37 @@ add_task(async function test_no_message_
 });
 
 add_task(async function test_prefd_off_message_on_navigation() {
   // Confirm the pref controls capture behavior with non-user-set field values.
   await SpecialPowers.pushPrefEnv({"set": [
     ["signon.userInputRequiredToCapture.enabled", false],
   ]});
   await setup(PREFILLED_FORM_URL);
-  await checkIframedUnmodifiedForm();
 
   let promiseSubmitMessage = getSubmitMessage();
   await clickLink();
   await promiseSubmitMessage;
   info("onFormSubmit message was sent as expected after navigation");
 
   SpecialPowers.popPrefEnv();
 });
 
 add_task(async function test_message_with_user_interaction_on_navigation() {
   await setup(PREFILLED_FORM_URL);
-  await checkIframedUnmodifiedForm();
   await userInput("#form-basic-username", "foo");
 
   let promiseSubmitMessage = getSubmitMessage();
   await clickLink();
   await promiseSubmitMessage;
   info("onFormSubmit message was sent as expected after user interaction");
 });
 
 add_task(async function test_empty_form_with_input_handler() {
   await setup(EXAMPLE_COM + "formless_basic.html");
-  await checkIframedUnmodifiedForm("body");
   await userInput("#form-basic-username", "user");
   await userInput("#form-basic-password", "pass");
 
   let promiseSubmitMessage = getSubmitMessage();
   await clickLink();
   await promiseSubmitMessage;
   info("onFormSubmit message was sent as expected after user interaction");
 });
@@ -197,17 +175,16 @@ add_task(async function test_message_on_
   ok(messageData.autoFilledLoginGuid, "Message was sent with autoFilledLoginGuid");
   is(messageData.usernameField.value, "newuser", "Message was sent with correct usernameField.value");
   info("Message was sent as expected after user interaction");
 });
 
 add_task(async function test_no_message_on_user_input_from_other_form() {
   // ensure input into unrelated fields on the page don't change login form modified-ness
   await setup(PREFILLED_FORM_URL);
-  await checkIframedUnmodifiedForm();
 
   // Add a form which will not be submitted and an input associated with that form
   await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
     let doc = this.content.document;
     let loginForm = doc.querySelector("form");
     let fragment = doc.createDocumentFragment();
     let otherForm = doc.createElement("form");
     otherForm.id ="otherForm";