Bug 1483412 - Make the telephone number field required on the payer form if requestPayerPhone is true. r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D10597

--HG--
extra : moz-landing-system : lando

browser/components/payments/res/containers/address-form.js

@@ -180,6 +180,11 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ
     } else {
       this.form.dataset.addressFields = "mailing-address tel";
     }
+    if (addressPage.selectedStateKey == "selectedPayerAddress") {
+      this.form.dataset.extraRequiredFields = addressPage.addressFields;
+    } else {
+      this.form.dataset.extraRequiredFields = "";
+    }
     this.formHandler.loadRecord(record);
 
     // Add validation to some address fields

browser/components/payments/res/containers/payment-dialog.js

@@ -304,7 +304,7 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     }
 
     if (payerRequested) {
-      let fieldNames = new Set(); // default: ["name", "tel", "email"]
+      let fieldNames = new Set();
       if (paymentOptions.requestPayerName) {
         fieldNames.add("name");
       }

browser/components/payments/res/unprivileged-fallbacks.js

@@ -75,6 +75,7 @@ var PaymentDialogUtils = {
           {fieldId: "address-level2"},
         ],
         postalCodePattern: "\\d{5}",
+        countryRequiredFields: ["street-address", "address-level2", "postal-code"],
       };
     }
 
@@ -94,6 +95,9 @@ var PaymentDialogUtils = {
       // The following values come from addressReferences.js and should not be changed.
       /* eslint-disable-next-line max-len */
       postalCodePattern: country == "US" ? "(\\d{5})(?:[ \\-](\\d{4}))?" : "[ABCEGHJKLMNPRSTVXY]\\d[ABCEGHJ-NPRSTV-Z] ?\\d[ABCEGHJ-NPRSTV-Z]\\d",
+      countryRequiredFields: country == "US" || country == "CA" ?
+        ["street-address", "address-level2", "address-level1", "postal-code"] :
+        ["street-address", "address-level2", "postal-code"],
     };
   },
   getDefaultPreferences() {

browser/components/payments/test/browser/browser.ini

@@ -17,6 +17,7 @@ skip-if = debug && (os == 'mac' || os == 'linux') # bug 1465673
 [browser_host_name.js]
 [browser_onboarding_wizard.js]
 [browser_openPreferences.js]
+[browser_payerRequestedFields.js]
 [browser_payment_completion.js]
 [browser_profile_storage.js]
 [browser_request_serialization.js]

browser/components/payments/test/browser/browser_payerRequestedFields.js

@@ -0,0 +1,108 @@
+/* eslint-disable no-shadow */
+
+"use strict";
+
+async function setup() {
+  await setupFormAutofillStorage();
+  await cleanupFormAutofillStorage();
+  // add an address and card to avoid the FTU sequence
+  let prefilledGuids = await addSampleAddressesAndBasicCard(
+    [PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
+
+  info("associating the card with the billing address");
+  await formAutofillStorage.creditCards.update(prefilledGuids.card1GUID, {
+    billingAddressGUID: prefilledGuids.address1GUID,
+  }, true);
+
+  return prefilledGuids;
+}
+
+/*
+ * Test that the payerRequested* fields are marked as required
+ * on the payer address form but aren't marked as required on
+ * the shipping address form.
+ */
+add_task(async function test_add_link() {
+  await setup();
+
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let {win, frame} =
+      await setupPaymentDialog(browser, {
+        methodData: [PTU.MethodData.basicCard],
+        details: Object.assign({}, PTU.Details.twoShippingOptions, PTU.Details.total2USD),
+        options: {...PTU.Options.requestShipping, ...PTU.Options.requestPayerNameEmailAndPhone},
+        merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
+      }
+    );
+
+    await navigateToAddAddressPage(frame, {
+      addLinkSelector: "address-picker.payer-related .add-link",
+      initialPageId: "payment-summary",
+      expectPersist: true,
+    });
+
+    await spawnPaymentDialogTask(frame, async () => {
+      let {
+        PaymentTestUtils,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      let title = content.document.querySelector("address-form h2");
+      is(title.textContent, "Add Payer Contact", "Page title should be set");
+
+      let saveButton = content.document.querySelector("address-form .save-button");
+      is(saveButton.textContent, "Next", "Save button has the correct label");
+
+      info("check that payer requested fields are marked as required");
+      for (let selector of ["#given-name", "#family-name", "#email", "#tel"]) {
+        let element = content.document.querySelector(selector);
+        ok(element.required, selector + " should be required");
+      }
+
+      let backButton = content.document.querySelector("address-form .back-button");
+      ok(content.isVisible(backButton),
+         "Back button is visible on the payer address page");
+      backButton.click();
+
+      await PaymentTestUtils.DialogContentUtils.waitForState(content, (state) => {
+        return state.page.id == "payment-summary";
+      }, "Switched back to payment-summary from payer address form");
+    });
+
+    await navigateToAddAddressPage(frame, {
+      addLinkSelector: "address-picker .add-link",
+      initialPageId: "payment-summary",
+      expectPersist: true,
+    });
+
+    await spawnPaymentDialogTask(frame, async () => {
+      let {
+        PaymentTestUtils,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      let title = content.document.querySelector("address-form h2");
+      is(title.textContent, "Add Shipping Address", "Page title should be set");
+
+      let saveButton = content.document.querySelector("address-form .save-button");
+      is(saveButton.textContent, "Next", "Save button has the correct label");
+
+      ok(!content.document.querySelector("#tel").required, "#tel should not be required");
+
+      let backButton = content.document.querySelector("address-form .back-button");
+      ok(content.isVisible(backButton),
+         "Back button is visible on the payer address page");
+      backButton.click();
+
+      await PaymentTestUtils.DialogContentUtils.waitForState(content, (state) => {
+        return state.page.id == "payment-summary";
+      }, "Switched back to payment-summary from payer address form");
+    });
+
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+  await cleanupFormAutofillStorage();
+});
+

browser/components/payments/test/mochitest/test_address_form.html

@@ -60,7 +60,7 @@ function checkAddressForm(customEl, expectedAddress) {
 function sendStringAndCheckValidity(element, string, isValid) {
   fillField(element, string);
   ok(element.checkValidity() == isValid,
-     `${element.id} should be ${isValid ? "valid" : "invalid"} (${string})`);
+     `${element.id} should be ${isValid ? "valid" : "invalid"} ("${string}")`);
 }
 
 add_task(async function test_initialState() {
@@ -278,8 +278,9 @@ add_task(async function test_edit() {
 
 add_task(async function test_restricted_address_fields() {
   let form = new AddressForm();
-  form.dataset.nextButtonLabel = "Next";
   form.dataset.errorGenericSave = "Generic error";
+  form.dataset.fieldRequiredSymbol = "*";
+  form.dataset.nextButtonLabel = "Next";
   await form.promiseReady;
   display.appendChild(form);
   await form.requestStore.setState({
@@ -312,15 +313,23 @@ add_task(async function test_restricted_address_fields() {
      "country should be hidden");
   ok(!isHidden(form.form.querySelector("#email")),
      "email should be visible");
-  ok(!isHidden(form.form.querySelector("#tel")),
+  let telField = form.form.querySelector("#tel");
+  ok(!isHidden(telField),
      "tel should be visible");
+  let telContainer = telField.closest(`#${telField.id}-container`);
+  ok(telContainer.hasAttribute("required"), "tel container should have required attribute");
+  let telSpan = telContainer.querySelector("span");
+  is(telSpan.getAttribute("fieldRequiredSymbol"), "*",
+     "tel span should have asterisk as fieldRequiredSymbol");
+  is(getComputedStyle(telSpan, "::after").content, "attr(fieldRequiredSymbol)",
+     "Asterisk should be on tel");
 
   fillField(form.form.querySelector("#given-name"), "John");
   fillField(form.form.querySelector("#family-name"), "Smith");
   ok(form.saveButton.disabled, "Save button should be disabled due to empty fields");
   fillField(form.form.querySelector("#email"), "john@example.com");
-  todo(form.saveButton.disabled,
-       "Save button should be disabled due to empty fields - Bug 1483412");
+  ok(form.saveButton.disabled,
+     "Save button should be disabled due to empty fields");
   fillField(form.form.querySelector("#tel"), "+15555555555");
   ok(!form.saveButton.disabled, "Save button should be enabled with all required fields filled");
 

browser/extensions/formautofill/FormAutofillUtils.jsm

@@ -451,6 +451,19 @@ this.FormAutofillUtils = {
     return this._collators[country];
   },
 
+  // Based on the list of fields abbreviations in
+  // https://github.com/googlei18n/libaddressinput/wiki/AddressValidationMetadata
+  FIELDS_LOOKUP: {
+    N: "name",
+    O: "organization",
+    A: "street-address",
+    S: "address-level1",
+    C: "address-level2",
+    D: "address-level3",
+    Z: "postal-code",
+    n: "newLine",
+  },
+
   /**
    * Parse a country address format string and outputs an array of fields.
    * Spaces, commas, and other literals are ignored in this implementation.
@@ -468,22 +481,10 @@ this.FormAutofillUtils = {
     if (!fmt) {
       throw new Error("fmt string is missing.");
     }
-    // Based on the list of fields abbreviations in
-    // https://github.com/googlei18n/libaddressinput/wiki/AddressValidationMetadata
-    const fieldsLookup = {
-      N: "name",
-      O: "organization",
-      A: "street-address",
-      S: "address-level1",
-      C: "address-level2",
-      D: "address-level3",
-      Z: "postal-code",
-      n: "newLine",
-    };
 
     return fmt.match(/%[^%]/g).reduce((parsed, part) => {
       // Take the first letter of each segment and try to identify it
-      let fieldId = fieldsLookup[part[1]];
+      let fieldId = this.FIELDS_LOOKUP[part[1]];
       // Early return if cannot identify part.
       if (!fieldId) {
         return parsed;
@@ -500,6 +501,23 @@ this.FormAutofillUtils = {
     }, []);
   },
 
+  /**
+   * Parse a require string and outputs an array of fields.
+   * Spaces, commas, and other literals are ignored in this implementation.
+   * For example, a require string "ACS" should return:
+   * ["street-address", "address-level2", "address-level1"]
+   *
+   * @param   {string} requireString Country address require string
+   * @returns {array<string>} List of fields
+   */
+  parseRequireString(requireString) {
+    if (!requireString) {
+      throw new Error("requireString string is missing.");
+    }
+
+    return requireString.split("").map(fieldId => this.FIELDS_LOOKUP[fieldId]);
+  },
+
   /**
    * Use alternative country name list to identify a country code from a
    * specified country name.
@@ -808,16 +826,15 @@ this.FormAutofillUtils = {
   getFormFormat(country) {
     const dataset = this.getCountryAddressData(country);
     return {
-      // Phillipines doesn't specify a sublocality_name_type but
-      // has one referenced in their fmt value.
+      // When particular values are missing for a country, the
+      // data/ZZ value should be used instead.
       addressLevel3Label: dataset.sublocality_name_type || "suburb",
-      // Many locales don't specify a locality_name_type but
-      // have one referenced in their fmt value.
       addressLevel2Label: dataset.locality_name_type || "city",
       addressLevel1Label: dataset.state_name_type || "province",
       postalCodeLabel: dataset.zip_name_type || "postalCode",
-      fieldsOrder: this.parseAddressFormat(dataset.fmt || "%N%n%O%n%A%n%C, %S %Z"),
+      fieldsOrder: this.parseAddressFormat(dataset.fmt || "%N%n%O%n%A%n%C"),
       postalCodePattern: dataset.zip,
+      countryRequiredFields: this.parseRequireString(dataset.require || "AC"),
     };
   },
 

browser/extensions/formautofill/content/autofillEditForms.js

@@ -213,14 +213,22 @@ class EditAddress extends EditAutofillForm {
       postalCodeLabel,
       fieldsOrder: mailingFieldsOrder,
       postalCodePattern,
+      countryRequiredFields,
     } = this.getFormFormat(country);
     this._elements.addressLevel3Label.dataset.localization = addressLevel3Label;
     this._elements.addressLevel2Label.dataset.localization = addressLevel2Label;
     this._elements.addressLevel1Label.dataset.localization = addressLevel1Label;
     this._elements.postalCodeLabel.dataset.localization = postalCodeLabel;
     let addressFields = this._elements.form.dataset.addressFields;
+    let extraRequiredFields = this._elements.form.dataset.extraRequiredFields;
     let fieldClasses = EditAddress.computeVisibleFields(mailingFieldsOrder, addressFields);
-    this.arrangeFields(fieldClasses);
+    let requiredFields = new Set(countryRequiredFields);
+    if (extraRequiredFields) {
+      for (let extraRequiredField of extraRequiredFields.trim().split(/\s+/)) {
+        requiredFields.add(extraRequiredField);
+      }
+    }
+    this.arrangeFields(fieldClasses, requiredFields);
     this.updatePostalCodeValidation(postalCodePattern);
   }
 
@@ -228,8 +236,9 @@ class EditAddress extends EditAutofillForm {
    * Update address field visibility and order based on libaddressinput data.
    *
    * @param {object[]} fieldsOrder array of objects with `fieldId` and optional `newLine` properties
+   * @param {Set} requiredFields Set of `fieldId` strings that mark which fields are required
    */
-  arrangeFields(fieldsOrder) {
+  arrangeFields(fieldsOrder, requiredFields) {
     let fields = [
       "name",
       "organization",
@@ -245,9 +254,18 @@ class EditAddress extends EditAutofillForm {
     let inputs = [];
     for (let i = 0; i < fieldsOrder.length; i++) {
       let {fieldId, newLine} = fieldsOrder[i];
+
       let container = this._elements.form.querySelector(`#${fieldId}-container`);
       let containerInputs = [...container.querySelectorAll("input, textarea, select")];
-      containerInputs.forEach(function(input) { input.disabled = false; });
+      containerInputs.forEach(function(input) {
+        input.disabled = false;
+        // libaddressinput doesn't list 'country' or 'name' as required.
+        // The additional-name field should never get marked as required.
+        input.required = (fieldId == "country" ||
+                          fieldId == "name" ||
+                          requiredFields.has(fieldId)) &&
+                         input.id != "additional-name";
+      });
       inputs.push(...containerInputs);
       container.style.display = "flex";
       container.style.order = i;

browser/extensions/formautofill/content/editAddress.xhtml

@@ -41,23 +41,23 @@
       <span data-localization="organization2" class="label-text"/>
     </label>
     <label id="street-address-container" class="container">
-      <textarea id="street-address" rows="3" required="required"/>
+      <textarea id="street-address" rows="3"/>
       <span data-localization="streetAddress" class="label-text"/>
     </label>
     <label id="address-level3-container" class="container">
-      <input id="address-level3" type="text" required="required"/>
+      <input id="address-level3" type="text"/>
       <span class="label-text"/>
     </label>
     <label id="address-level2-container" class="container">
-      <input id="address-level2" type="text" required="required"/>
+      <input id="address-level2" type="text"/>
       <span class="label-text"/>
     </label>
     <label id="address-level1-container" class="container">
-      <input id="address-level1" type="text" required="required"/>
+      <input id="address-level1" type="text"/>
       <span class="label-text"/>
     </label>
     <label id="postal-code-container" class="container">
-      <input id="postal-code" type="text" required="required"/>
+      <input id="postal-code" type="text"/>
       <span class="label-text"/>
     </label>
     <label id="country-container" class="container">