Bug 1483425 - Don't mark form fields as invalid/dirty on load in an 'add' form. r=sfoster
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Fri, 17 Aug 2018 16:27:21 -0700
changeset 487330 457c223707df08866c3abd734781f75500dd1412
parent 487329 c99d9b6ba765993b58f8570e9ad62d5f5dbcea1a
child 487331 8a7bbdede1c93456d7b2a30bff3c646463aad0e3
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster
bugs1483425
milestone63.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 1483425 - Don't mark form fields as invalid/dirty on load in an 'add' form. r=sfoster When editing an existing record, invalid fields should be indicated on load. Differential Revision: https://phabricator.services.mozilla.com/D3397 MozReview-Commit-ID: CbrqEyCUoyC
browser/components/payments/test/mochitest/test_address_form.html
browser/components/payments/test/mochitest/test_basic_card_form.html
browser/extensions/formautofill/content/autofillEditForms.js
--- a/browser/components/payments/test/mochitest/test_address_form.html
+++ b/browser/components/payments/test/mochitest/test_address_form.html
@@ -67,16 +67,21 @@ function sendStringAndCheckValidity(elem
 add_task(async function test_initialState() {
   let form = new AddressForm();
   let {page} = form.requestStore.getState();
   is(page.id, "payment-summary", "Check initial page");
   await form.promiseReady;
   display.appendChild(form);
   await asyncElementRendered();
   is(page.id, "payment-summary", "Check initial page after appending");
+
+  // :-moz-ui-invalid, unlike :invalid, only applies to fields showing the error outline.
+  let fieldsVisiblyInvalid = form.querySelectorAll(":-moz-ui-invalid");
+  is(fieldsVisiblyInvalid.length, 0, "Check no fields are visibly invalid on an empty 'add' form");
+
   form.remove();
 });
 
 add_task(async function test_backButton() {
   let form = new AddressForm();
   form.dataset.backButtonLabel = "Back";
   await form.requestStore.setState({
     page: {
@@ -123,18 +128,23 @@ add_task(async function test_saveButton(
   fillField(form.form.querySelector("#email"), "test@example.com");
   fillField(form.form.querySelector("#tel"), "+15555551212");
 
   ok(!form.saveButton.disabled, "Save button is enabled after filling");
 
   info("blanking the street-address");
   fillField(form.form.querySelector("#street-address"), "");
   ok(form.saveButton.disabled, "Save button is disabled after blanking street-address");
+  form.form.querySelector("#street-address").blur();
+  let fieldsVisiblyInvalid = form.querySelectorAll(":-moz-ui-invalid");
+  is(fieldsVisiblyInvalid.length, 1, "Check 1 field visibly invalid after blanking and blur");
+  is(fieldsVisiblyInvalid[0].id, "street-address", "Check #street-address is visibly invalid");
 
   fillField(form.form.querySelector("#street-address"), "404 Internet Super Highway");
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0, "Check no fields visibly invalid");
   ok(!form.saveButton.disabled, "Save button is enabled after re-filling street-address");
 
   let messagePromise = promiseContentToChromeMessage("updateAutofillRecord");
   is(form.saveButton.textContent, "Add", "Check label");
   form.saveButton.scrollIntoView();
   synthesizeMouseAtCenter(form.saveButton, {});
 
   let details = await messagePromise;
@@ -195,16 +205,18 @@ add_task(async function test_edit() {
     "address-page": {
       guid: address1.guid,
     },
     savedAddresses: {
       [address1.guid]: deepClone(address1),
     },
   });
   await asyncElementRendered();
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0,
+     "Check no fields are visibly invalid on an 'edit' form with a complete address");
   checkAddressForm(form, address1);
 
   ok(!form.saveButton.disabled, "Save button should be enabled upon edit for a valid address");
 
   info("test change to minimal record");
   let minimalAddress = {
     "given-name": address1["given-name"],
     guid: "9gnjdhen46",
@@ -219,25 +231,31 @@ add_task(async function test_edit() {
     savedAddresses: {
       [minimalAddress.guid]: deepClone(minimalAddress),
     },
   });
   await asyncElementRendered();
   is(form.saveButton.textContent, "Update", "Check label");
   checkAddressForm(form, minimalAddress);
   ok(form.saveButton.disabled, "Save button should be disabled if only the name is filled");
+  ok(form.querySelectorAll(":-moz-ui-invalid").length > 3,
+     "Check fields are visibly invalid on an 'edit' form with only the given-name filled");
+  ok(form.querySelectorAll("#country:-moz-ui-invalid").length, 1,
+     "Check that the country `select` is marked as invalid");
 
   info("change to no selected address");
   await form.requestStore.setState({
     page: {
       id: "address-page",
     },
     "address-page": {},
   });
   await asyncElementRendered();
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0,
+     "Check no fields are visibly invalid on an empty 'add' form after being an edit form");
   checkAddressForm(form, {});
   ok(form.saveButton.disabled, "Save button should be disabled for an empty form");
 
   form.remove();
 });
 
 add_task(async function test_restricted_address_fields() {
   let form = new AddressForm();
@@ -350,17 +368,17 @@ add_task(async function test_field_valid
   sendStringAndCheckValidity(addressLevel1Input, "", false);
   sendStringAndCheckValidity(postalCodeInput, "11109", false);
   sendStringAndCheckValidity(addressLevel1Input, "Nova Scotia", true);
   sendStringAndCheckValidity(postalCodeInput, "06390-0001", false);
 
   form.remove();
 });
 
-add_task(async function test_customValidity() {
+add_task(async function test_customMerchantValidity() {
   let form = new AddressForm();
   await form.promiseReady;
   const state = {
     page: {
       id: "address-page",
     },
     "address-page": {
       title: "Sample page title",
@@ -401,16 +419,59 @@ add_task(async function test_customValid
   checkValidationMessage("#given-name", "recipient");
   checkValidationMessage("#address-level1", "region");
 
   // TODO: bug 1482808 - the save button should be enabled after editing the fields
 
   form.remove();
 });
 
+add_task(async function test_customMerchantValidity_reset() {
+  let form = new AddressForm();
+  await form.promiseReady;
+  const state = {
+    page: {
+      id: "address-page",
+    },
+    "address-page": {
+      title: "Sample page title",
+    },
+    request: {
+      paymentDetails: {
+        shippingAddressErrors: {
+          addressLine: "Street address needs to start with a D",
+          city: "City needs to start with a B",
+          country: "Country needs to start with a C",
+          organization: "organization needs to start with an A",
+          phone: "Telephone needs to start with a 9",
+          postalCode: "Postal code needs to start with a 0",
+          recipient: "Name needs to start with a Z",
+          region: "Region needs to start with a Y",
+        },
+      },
+    },
+  };
+  await form.requestStore.setState(state);
+  display.appendChild(form);
+  await asyncElementRendered();
+
+  ok(form.querySelectorAll(":-moz-ui-invalid").length > 0, "Check fields are visibly invalid");
+  info("merchant cleared the errors");
+  await form.requestStore.setState({
+    request: {
+      paymentDetails: {
+        shippingAddressErrors: {},
+      },
+    },
+  });
+  await asyncElementRendered();
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0,
+     "Check fields are visibly valid - custom validity cleared");
+});
+
 add_task(async function test_field_validation() {
   sinon.stub(PaymentDialogUtils, "getFormFormat").returns({
     addressLevel1Label: "state",
     postalCodeLabel: "US",
     fieldsOrder: [
       {fieldId: "name", newLine: true},
       {fieldId: "organization", newLine: true},
       {fieldId: "street-address", newLine: true},
--- a/browser/components/payments/test/mochitest/test_basic_card_form.html
+++ b/browser/components/payments/test/mochitest/test_basic_card_form.html
@@ -54,16 +54,21 @@ function checkCCForm(customEl, expectedC
 add_task(async function test_initialState() {
   let form = new BasicCardForm();
   let {page} = form.requestStore.getState();
   is(page.id, "payment-summary", "Check initial page");
   await form.promiseReady;
   display.appendChild(form);
   await asyncElementRendered();
   is(page.id, "payment-summary", "Check initial page after appending");
+
+  // :-moz-ui-invalid, unlike :invalid, only applies to fields showing the error outline.
+  let fieldsVisiblyInvalid = form.querySelectorAll(":-moz-ui-invalid");
+  is(fieldsVisiblyInvalid.length, 0, "Check no fields are visibly invalid on an empty 'add' form");
+
   form.remove();
 });
 
 add_task(async function test_backButton() {
   let form = new BasicCardForm();
   form.dataset.backButtonLabel = "Back";
   form.dataset.addBasicCardTitle = "Sample page title 2";
   await form.requestStore.setState({
@@ -92,31 +97,40 @@ add_task(async function test_saveButton(
   let form = new BasicCardForm();
   form.dataset.addButtonLabel = "Add";
   form.dataset.errorGenericSave = "Generic error";
   await form.promiseReady;
   display.appendChild(form);
   await asyncElementRendered();
 
   ok(form.saveButton.disabled, "Save button should initially be disabled");
-  form.form.querySelector("#cc-number").focus();
-  sendString("4111 1111-1111 1111");
+  fillField(form.form.querySelector("#cc-number"), "4111 1111-1111 1111");
   form.form.querySelector("#cc-name").focus();
   // Check .disabled after .focus() so that it's after both "input" and "change" events.
   ok(form.saveButton.disabled, "Save button should still be disabled without a name");
   sendString("J. Smith");
-  form.form.querySelector("#cc-exp-month").focus();
-  sendString("11");
-  form.form.querySelector("#cc-exp-year").focus();
+  fillField(form.form.querySelector("#cc-exp-month"), "11");
   let year = (new Date()).getFullYear().toString();
-  sendString(year);
+  fillField(form.form.querySelector("#cc-exp-year"), year);
   form.saveButton.focus();
   ok(!form.saveButton.disabled,
      "Save button should be enabled since the required fields are filled");
 
+  info("blanking the cc-number field");
+  fillField(form.form.querySelector("#cc-number"), "");
+  ok(form.saveButton.disabled, "Save button is disabled after blanking cc-number");
+  form.form.querySelector("#cc-number").blur();
+  let fieldsVisiblyInvalid = form.querySelectorAll(":-moz-ui-invalid");
+  is(fieldsVisiblyInvalid.length, 1, "Check 1 field visibly invalid after blanking and blur");
+  is(fieldsVisiblyInvalid[0].id, "cc-number", "Check #cc-number is visibly invalid");
+
+  fillField(form.form.querySelector("#cc-number"), "4111 1111-1111 1111");
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0, "Check no fields visibly invalid");
+  ok(!form.saveButton.disabled, "Save button is enabled after re-filling cc-number");
+
   let messagePromise = promiseContentToChromeMessage("updateAutofillRecord");
   is(form.saveButton.textContent, "Add", "Check label");
   synthesizeMouseAtCenter(form.saveButton, {});
 
   let details = await messagePromise;
   ok(typeof(details.messageID) == "number" && details.messageID > 0, "Check messageID type");
   delete details.messageID;
   is(details.collectionName, "creditCards", "Check collectionName");
@@ -287,17 +301,20 @@ add_task(async function test_edit() {
       guid: card1.guid,
     },
     savedBasicCards: {
       [card1.guid]: deepClone(card1),
     },
   });
   await asyncElementRendered();
   is(form.saveButton.textContent, "Update", "Check label");
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0,
+     "Check no fields are visibly invalid on an 'edit' form with a complete card");
   checkCCForm(form, card1);
+  ok(!form.saveButton.disabled, "Save button should be enabled upon edit for a valid card");
 
   let requiredElements = [...form.form.elements].filter(e => e.required && !e.disabled);
   ok(requiredElements.length, "There should be at least one required element");
   for (let element of requiredElements) {
     let container = element.closest("label") || element.closest("div");
     ok(element.hasAttribute("required"), "Element should be marked as required");
     ok(container.hasAttribute("required"), "Container should also be marked as required");
   }
@@ -326,75 +343,103 @@ add_task(async function test_edit() {
     "basic-card-page": {
       guid: minimalCard.guid,
     },
     savedBasicCards: {
       [minimalCard.guid]: deepClone(minimalCard),
     },
   });
   await asyncElementRendered();
+  ok(form.querySelectorAll(":-moz-ui-invalid").length > 0,
+     "Check fields are visibly invalid on an 'edit' form with missing fields");
   checkCCForm(form, minimalCard);
 
   info("change to no selected card");
   await form.requestStore.setState({
     page: {
       id: "basic-card-page",
     },
     "basic-card-page": {
       guid: null,
     },
   });
   await asyncElementRendered();
+  is(form.querySelectorAll(":-moz-ui-invalid").length, 0,
+     "Check no fields are visibly invalid after converting to an 'add' form");
   checkCCForm(form, {});
 
   form.remove();
 });
 
 add_task(async function test_field_validity_updates() {
   let form = new BasicCardForm();
   form.dataset.updateButtonLabel = "Add";
   await form.promiseReady;
   display.appendChild(form);
   await asyncElementRendered();
 
   let ccNumber = form.form.querySelector("#cc-number");
   let nameInput = form.form.querySelector("#cc-name");
 
   info("test with valid cc-number but missing cc-name");
-  ccNumber.focus();
-  sendString("4111111111111111");
+  fillField(ccNumber, "4111111111111111");
   ok(ccNumber.checkValidity(), "cc-number field is valid with good input");
   ok(!nameInput.checkValidity(), "cc-name field is invalid when empty");
   ok(form.saveButton.disabled, "Save button should be disabled with incomplete input");
 
   info("correct by adding cc-name value");
-  nameInput.focus();
-  sendString("First");
+  fillField(nameInput, "First");
   ok(ccNumber.checkValidity(), "cc-number field is valid with good input");
   ok(nameInput.checkValidity(), "cc-name field is valid with a value");
   ok(!form.saveButton.disabled, "Save button should not be disabled with good input");
 
   info("edit to make the cc-number invalid");
   ccNumber.focus();
   sendString("aa");
   nameInput.focus();
   sendString("Surname");
 
   ok(!ccNumber.checkValidity(), "cc-number field becomes invalid with bad input");
+  ok(form.querySelector("#cc-number:-moz-ui-invalid"), "cc-number field is visibly invalid");
   ok(nameInput.checkValidity(), "cc-name field is valid with a value");
   ok(form.saveButton.disabled, "Save button becomes disabled with bad input");
 
   info("fix the cc-number to make it all valid again");
   ccNumber.focus();
   sendKey("BACK_SPACE");
   sendKey("BACK_SPACE");
   info("after backspaces, ccNumber.value: " + ccNumber.value);
 
   ok(ccNumber.checkValidity(), "cc-number field becomes valid with corrected input");
   ok(nameInput.checkValidity(), "cc-name field is valid with a value");
   ok(!form.saveButton.disabled, "Save button is no longer disabled with corrected input");
 
   form.remove();
 });
+
+add_task(async function test_numberCustomValidityReset() {
+  let form = new BasicCardForm();
+  form.dataset.updateButtonLabel = "Add";
+  await form.promiseReady;
+  display.appendChild(form);
+  await asyncElementRendered();
+
+  fillField(form.querySelector("#cc-number"), "junk");
+  sendKey("TAB");
+  ok(form.querySelector("#cc-number:-moz-ui-invalid"), "cc-number field is visibly invalid");
+
+  info("simulate triggering an add again to reset the form");
+  await form.requestStore.setState({
+    page: {
+      id: "basic-card-page",
+    },
+    "basic-card-page": {
+    },
+  });
+
+  ok(!form.querySelector("#cc-number:-moz-ui-invalid"), "cc-number field is not visibly invalid");
+
+  form.remove();
+});
 </script>
 
 </body>
 </html>
--- a/browser/extensions/formautofill/content/autofillEditForms.js
+++ b/browser/extensions/formautofill/content/autofillEditForms.js
@@ -14,17 +14,37 @@ class EditAutofillForm {
 
   /**
    * Fill the form with a record object.
    * @param  {object} [record = {}]
    */
   loadRecord(record = {}) {
     for (let field of this._elements.form.elements) {
       let value = record[field.id];
-      field.value = typeof(value) == "undefined" ? "" : value;
+      value = typeof(value) == "undefined" ? "" : value;
+
+      if (record.guid) {
+        field.value = value;
+      } else if (field.localName == "select") {
+        this.setDefaultSelectedOptionByValue(field, value);
+      } else {
+        // Use .defaultValue instead of .value to avoid setting the `dirty` flag
+        // which triggers form validation UI.
+        field.defaultValue = value;
+      }
+    }
+    if (!record.guid) {
+      // Reset the dirty value flag and validity state.
+      this._elements.form.reset();
+    }
+  }
+
+  setDefaultSelectedOptionByValue(select, value) {
+    for (let option of select.options) {
+      option.defaultSelected = option.value == value;
     }
   }
 
   /**
    * Get inputs from the form.
    * @returns {object}
    */
   buildFormObject() {
@@ -277,16 +297,21 @@ class EditCreditCard extends EditAutofil
     // _record must be updated before generateYears and generateBillingAddressOptions are called.
     this._record = record;
     this._addresses = addresses;
     this.generateBillingAddressOptions();
     if (!preserveFieldValues) {
       // Re-generating the years will reset the selected option.
       this.generateYears();
       super.loadRecord(record);
+
+      // Resetting the form in the super.loadRecord won't clear custom validity
+      // state so reset it here. Since the cc-number field is disabled upon editing
+      // we don't need to recaclulate its validity here.
+      this._elements.ccNumber.setCustomValidity("");
     }
   }
 
   generateYears() {
     const count = 11;
     const currentYear = new Date().getFullYear();
     const ccExpYear = this._record && this._record["cc-exp-year"];