From a290cf90f34b01c94aa118dcf685c48b48bab50b Mon Sep 17 00:00:00 2001 From: Matthew Noorenberghe Date: Wed, 15 Aug 2018 12:19:55 -0700 Subject: [PATCH] Bug 1476345 - Only enable relevant fields in address forms and update tests. r=jaws MozReview-Commit-ID: KuPMHrF6jaM --HG-- extra : rebase_source : f37118ff94bcb90108712dcc2f6db3d0aa5c92ef --- .../payments/res/containers/address-form.css | 17 ---- .../payments/res/containers/address-form.js | 13 +-- .../payments/res/containers/address-picker.js | 4 +- .../payments/res/containers/form.css | 1 + .../payments/test/PaymentTestUtils.jsm | 2 +- .../test/browser/browser_address_edit.js | 16 ++-- .../test/browser/browser_card_edit.js | 22 ++--- .../test/browser/browser_change_shipping.js | 1 + .../browser_payments_onboarding_wizard.js | 75 ++++++++-------- .../browser_shippingaddresschange_error.js | 14 +++ .../components/payments/test/browser/head.js | 76 +++++++++++++--- .../test/mochitest/payments_common.js | 26 +++++- .../test/mochitest/test_address_form.html | 87 ++++++++++++------- .../formautofill/content/autofillEditForms.js | 61 ++++++++++++- .../formautofill/content/editAddress.xhtml | 42 ++++----- .../formautofill/skin/shared/editAddress.css | 3 +- .../test/browser/browser_editAddressDialog.js | 12 +-- 17 files changed, 319 insertions(+), 153 deletions(-) diff --git a/browser/components/payments/res/containers/address-form.css b/browser/components/payments/res/containers/address-form.css index 72e72fa82a99..1c537b948525 100644 --- a/browser/components/payments/res/containers/address-form.css +++ b/browser/components/payments/res/containers/address-form.css @@ -7,23 +7,6 @@ display: none; } -/* Hide all form fields that are not explicitly requested - * by the paymentOptions object. - */ -address-form[address-fields]:not([address-fields~='name']) #name-container, -address-form[address-fields] #organization-container, -address-form[address-fields] #street-address-container, -address-form[address-fields] #address-level2-container, -address-form[address-fields] #address-level1-container, -address-form[address-fields] #postal-code-container, -address-form[address-fields] #country-container, -address-form[address-fields]:not([address-fields~='email']) #email-container, -address-form[address-fields]:not([address-fields~='tel']) #tel-container { - /* !important is needed because autofillEditForms.js sets - inline styles on the form fields with display: flex; */ - display: none !important; -} - .error-text:not(:empty) { color: #fff; background-color: #d70022; diff --git a/browser/components/payments/res/containers/address-form.js b/browser/components/payments/res/containers/address-form.js index aceda14db773..6a0468bbefbd 100644 --- a/browser/components/payments/res/containers/address-form.js +++ b/browser/components/payments/res/containers/address-form.js @@ -25,6 +25,8 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ super(); this.genericErrorText = document.createElement("div"); + this.genericErrorText.setAttribute("aria-live", "polite"); + this.genericErrorText.classList.add("page-error"); this.cancelButton = document.createElement("button"); this.cancelButton.className = "cancel-button"; @@ -129,12 +131,6 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ this.backButton.hidden = page.onboardingWizard; this.cancelButton.hidden = !page.onboardingWizard; - if (addressPage.addressFields) { - this.setAttribute("address-fields", addressPage.addressFields); - } else { - this.removeAttribute("address-fields"); - } - this.pageTitleHeading.textContent = addressPage.title; this.genericErrorText.textContent = page.error; @@ -160,6 +156,11 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ saveAddressDefaultChecked; } + if (addressPage.addressFields) { + this.form.dataset.addressFields = addressPage.addressFields; + } else { + this.form.dataset.addressFields = "mailing-address tel"; + } this.formHandler.loadRecord(record); // Add validation to some address fields diff --git a/browser/components/payments/res/containers/address-picker.js b/browser/components/payments/res/containers/address-picker.js index f3fe4042a292..722368376d21 100644 --- a/browser/components/payments/res/containers/address-picker.js +++ b/browser/components/payments/res/containers/address-picker.js @@ -31,14 +31,14 @@ export default class AddressPicker extends RichPicker { get fieldNames() { if (this.hasAttribute("address-fields")) { - let names = this.getAttribute("address-fields").split(/\s+/); + let names = this.getAttribute("address-fields").trim().split(/\s+/); if (names.length) { return names; } } return [ - "address-level1", + // "address-level1", // TODO: bug 1481481 - not required for some countries e.g. DE "address-level2", "country", "name", diff --git a/browser/components/payments/res/containers/form.css b/browser/components/payments/res/containers/form.css index ac32e05acf7c..f5f81bb5811b 100644 --- a/browser/components/payments/res/containers/form.css +++ b/browser/components/payments/res/containers/form.css @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +div[required] > label > span:first-of-type::after, :-moz-any(label, div)[required] > span:first-of-type::after { content: attr(fieldRequiredSymbol); } diff --git a/browser/components/payments/test/PaymentTestUtils.jsm b/browser/components/payments/test/PaymentTestUtils.jsm index 925d27a2174f..f66b3f8be927 100644 --- a/browser/components/payments/test/PaymentTestUtils.jsm +++ b/browser/components/payments/test/PaymentTestUtils.jsm @@ -448,7 +448,7 @@ var PaymentTestUtils = { organization: "World Wide Web Consortium", "street-address": "1 Pommes Frittes Place", "address-level2": "Berlin", - "address-level1": "BE", + // address-level1 isn't used in our forms for Germany "postal-code": "02138", country: "DE", tel: "+16172535702", diff --git a/browser/components/payments/test/browser/browser_address_edit.js b/browser/components/payments/test/browser/browser_address_edit.js index 5666fc89f577..bf5aeddb081a 100644 --- a/browser/components/payments/test/browser/browser_address_edit.js +++ b/browser/components/payments/test/browser/browser_address_edit.js @@ -51,14 +51,16 @@ add_task(async function test_add_link() { { setPersistCheckedValue: true, expectPersist: true }, { setPersistCheckedValue: false, expectPersist: false }, ]; - let newAddress = PTU.Addresses.TimBL2; + let newAddress = Object.assign({}, PTU.Addresses.TimBL2); + // Emails aren't part of shipping addresses + delete newAddress.email; for (let options of testOptions) { let shippingAddressChangePromise = ContentTask.spawn(browser, { eventName: "shippingaddresschange", }, PTU.ContentTasks.awaitPaymentRequestEventPromise); - await manuallyAddAddress(frame, newAddress, options); + await manuallyAddShippingAddress(frame, newAddress, options); await shippingAddressChangePromise; info("got shippingaddresschange event"); @@ -147,7 +149,7 @@ add_task(async function test_edit_link() { isEditing: true, expectPersist: true, }; - await fillInAddressForm(frame, EXPECTED_ADDRESS, editOptions); + await fillInShippingAddressForm(frame, EXPECTED_ADDRESS, editOptions); await verifyPersistCheckbox(frame, editOptions); await submitAddressForm(frame, EXPECTED_ADDRESS, editOptions); @@ -240,7 +242,7 @@ add_task(async function test_add_payer_contact_name_email_link() { } }); - await fillInAddressForm(frame, EXPECTED_ADDRESS, addOptions); + await fillInPayerAddressForm(frame, EXPECTED_ADDRESS, addOptions); await verifyPersistCheckbox(frame, addOptions); await submitAddressForm(frame, EXPECTED_ADDRESS, addOptions); @@ -397,7 +399,9 @@ add_task(async function test_private_persist_addresses() { ); info("/setupPaymentDialog"); - let addressToAdd = PTU.Addresses.Temp; + let addressToAdd = Object.assign({}, PTU.Addresses.Temp); + // Emails aren't part of shipping addresses + delete addressToAdd.email; const addOptions = { checkboxSelector: "#address-page .persist-checkbox", expectPersist: false, @@ -422,7 +426,7 @@ add_task(async function test_private_persist_addresses() { is(initialAddresses.options.length, 1, "Got expected number of pre-filled shipping addresses"); - await fillInAddressForm(frame, addressToAdd, addOptions); + await fillInShippingAddressForm(frame, addressToAdd, addOptions); await verifyPersistCheckbox(frame, addOptions); await submitAddressForm(frame, addressToAdd, addOptions); diff --git a/browser/components/payments/test/browser/browser_card_edit.js b/browser/components/payments/test/browser/browser_card_edit.js index aac1a1d1311a..fd6a8f87e409 100644 --- a/browser/components/payments/test/browser/browser_card_edit.js +++ b/browser/components/payments/test/browser/browser_card_edit.js @@ -30,7 +30,7 @@ async function add_link(aOptions = {}) { checkboxSelector: "basic-card-form .persist-checkbox", expectPersist: aOptions.expectDefaultCardPersist, }); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function checkState(testArgs = {}) { let { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); @@ -58,7 +58,7 @@ async function add_link(aOptions = {}) { await verifyPersistCheckbox(frame, cardOptions); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function checkBillingAddressPicker(testArgs = {}) { let billingAddressSelect = content.document.querySelector("#billingAddressGUID"); ok(content.isVisible(billingAddressSelect), "The billing address selector should always be visible"); @@ -82,7 +82,7 @@ async function add_link(aOptions = {}) { await navigateToAddAddressPage(frame, addressOptions); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function checkTask(testArgs = {}) { let { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); @@ -121,13 +121,13 @@ async function add_link(aOptions = {}) { await navigateToAddAddressPage(frame, addressOptions); - await fillInAddressForm(frame, PTU.Addresses.TimBL2, addressOptions); + await fillInBillingAddressForm(frame, PTU.Addresses.TimBL2, addressOptions); await verifyPersistCheckbox(frame, addressOptions); await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function checkCardPage(testArgs = {}) { let { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); @@ -168,7 +168,7 @@ async function add_link(aOptions = {}) { await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function waitForSummaryPage(testArgs = {}) { let { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); @@ -182,7 +182,7 @@ async function add_link(aOptions = {}) { securityCode: "123", }); - await spawnPaymentDialogTask(frame, async (testArgs = {}) => { + await spawnPaymentDialogTask(frame, async function checkCardState(testArgs = {}) { let { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); @@ -476,13 +476,15 @@ add_task(async function test_edit_link() { return state.page.id == "address-page" && state["address-page"].guid; }, "Check address page state (editing)"); - info("filling address fields"); - for (let [key, val] of Object.entries(PTU.Addresses.TimBL)) { + info("modify some address fields"); + for (let key of ["given-name", "tel", "organization", "street-address"]) { let field = content.document.getElementById(key); if (!field) { ok(false, `${key} field not found`); } - field.value = val.slice(0, -1) + "7"; + field.focus(); + EventUtils.sendKey("BACK_SPACE", content.window); + EventUtils.sendString("7", content.window); ok(!field.disabled, `Field #${key} shouldn't be disabled`); } diff --git a/browser/components/payments/test/browser/browser_change_shipping.js b/browser/components/payments/test/browser/browser_change_shipping.js index c99988e48406..f425d21771c0 100644 --- a/browser/components/payments/test/browser/browser_change_shipping.js +++ b/browser/components/payments/test/browser/browser_change_shipping.js @@ -100,6 +100,7 @@ add_task(async function test_change_shipping() { is(items.length, 1, "1 additional display item"); is(items[0].amountCurrency, "EUR", "First display item is in Euros"); is(items[0].amountValue, "1.00", "First display item has 1.00 value"); + btn.click(); }); info("clicking pay"); diff --git a/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js b/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js index 389167f31cdc..c34e4bfe0329 100644 --- a/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js +++ b/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js @@ -59,16 +59,15 @@ add_task(async function test_onboarding_wizard_without_saved_addresses_and_saved let addressCancelButton = content.document.querySelector("address-form .cancel-button"); ok(content.isVisible(addressCancelButton), "The cancel button on the address page is visible"); + }); - for (let [key, val] of Object.entries(PTU.Addresses.TimBL2)) { - let field = content.document.getElementById(key); - if (!field) { - ok(false, `${key} field not found`); - } - field.value = val; - ok(!field.disabled, `Field #${key} shouldn't be disabled`); - } - content.document.querySelector("address-form .save-button").click(); + await fillInShippingAddressForm(frame, PTU.Addresses.TimBL2); + await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); + + await spawnPaymentDialogTask(frame, async function() { + let { + PaymentTestUtils: PTU, + } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); await PTU.DialogContentUtils.waitForState(content, (state) => { return state.page.id == "basic-card-page"; @@ -271,16 +270,16 @@ add_task(async function test_onboarding_wizard_without_saved_address_with_saved_ info("Checking if the address page has been rendered"); let addressSaveButton = content.document.querySelector("address-form .save-button"); ok(content.isVisible(addressSaveButton), "Address save button is rendered"); + }); - for (let [key, val] of Object.entries(PTU.Addresses.TimBL2)) { - let field = content.document.getElementById(key); - if (!field) { - ok(false, `${key} field not found`); - } - field.value = val; - ok(!field.disabled, `Field #${key} shouldn't be disabled`); - } - content.document.querySelector("address-form .save-button").click(); + + await fillInShippingAddressForm(frame, PTU.Addresses.TimBL2); + await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); + + await spawnPaymentDialogTask(frame, async function checkSavedAndCancelButton() { + let { + PaymentTestUtils: PTU, + } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); await PTU.DialogContentUtils.waitForState(content, (state) => { return state.page.id == "payment-summary"; @@ -334,16 +333,15 @@ add_task(async function test_onboarding_wizard_with_requestShipping_turned_off() ok(content.isVisible(addressPageTitle), "Address page title is visible"); is(addressPageTitle.textContent, "Add Billing Address", "Address page title is correctly shown"); + }); - for (let [key, val] of Object.entries(PTU.Addresses.TimBL2)) { - let field = content.document.getElementById(key); - if (!field) { - ok(false, `${key} field not found`); - } - field.value = val; - ok(!field.disabled, `Field #${key} shouldn't be disabled`); - } - content.document.querySelector("address-form .save-button").click(); + await fillInBillingAddressForm(frame, PTU.Addresses.TimBL2); + await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); + + await spawnPaymentDialogTask(frame, async function() { + let { + PaymentTestUtils: PTU, + } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); await PTU.DialogContentUtils.waitForState(content, (state) => { return state.page.id == "basic-card-page"; @@ -443,16 +441,17 @@ add_task(async function test_back_button_on_basic_card_page_during_onboarding() info("Checking if the address page has been rendered"); let addressSaveButton = content.document.querySelector("address-form .save-button"); ok(content.isVisible(addressSaveButton), "Address save button is rendered"); + }); - for (let [key, val] of Object.entries(PTU.Addresses.TimBL2)) { - let field = content.document.getElementById(key); - if (!field) { - ok(false, `${key} field not found`); - } - field.value = val; - ok(!field.disabled, `Field #${key} shouldn't be disabled`); - } - content.document.querySelector("address-form .save-button").click(); + await fillInBillingAddressForm(frame, PTU.Addresses.TimBL2); + await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton); + + await spawnPaymentDialogTask(frame, async function() { + let { + PaymentTestUtils: PTU, + } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); + + let addressSaveButton = content.document.querySelector("address-form .save-button"); await PTU.DialogContentUtils.waitForState(content, (state) => { return state.page.id == "basic-card-page"; @@ -464,7 +463,7 @@ add_task(async function test_back_button_on_basic_card_page_during_onboarding() info("Partially fill basic card form"); let field = content.document.getElementById("cc-number"); - field.value = PTU.BasicCards.JohnDoe["cc-number"]; + content.fillField(field, PTU.BasicCards.JohnDoe["cc-number"]); info("Clicking on the back button to edit address saved in the previous step"); basicCardBackButton.click(); @@ -484,7 +483,7 @@ add_task(async function test_back_button_on_basic_card_page_during_onboarding() "Given name field value is correctly loaded"); info("Editing the address and saving again"); - field.value = "John"; + content.fillField(field, "John"); addressSaveButton.click(); info("Checking if the address was correctly edited"); diff --git a/browser/components/payments/test/browser/browser_shippingaddresschange_error.js b/browser/components/payments/test/browser/browser_shippingaddresschange_error.js index 173ed9d0bf38..aed537e3fc26 100644 --- a/browser/components/payments/test/browser/browser_shippingaddresschange_error.js +++ b/browser/components/payments/test/browser/browser_shippingaddresschange_error.js @@ -154,6 +154,20 @@ add_task(async function test_show_field_specific_error_on_addresschange() { PaymentTestUtils: PTU, } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); + // TODO: bug 1482808 - Clear setCustomValidity from merchant errors since + // they don't currently ever get cleared. + for (let field of content.document.querySelector("address-form form").elements) { + if (!field.validity.customError) { + continue; + } + field.setCustomValidity(""); + todo(false, `Clearing custom validity on #${field.id}`); + } + + Cu.waiveXrays(content.document.querySelector("address-form")).updateSaveButtonState(); + + // End bug 1482808 TODO + info("saving corrections"); content.document.querySelector("address-form .save-button").click(); diff --git a/browser/components/payments/test/browser/head.js b/browser/components/payments/test/browser/head.js index b484fbf05e16..46f7635fe364 100644 --- a/browser/components/payments/test/browser/head.js +++ b/browser/components/payments/test/browser/head.js @@ -199,7 +199,7 @@ function checkPaymentAddressMatchesStorageAddress(paymentAddress, storageAddress is(paymentAddress.addressLine[0], addressLines[0], "Address line 1 should match"); is(paymentAddress.addressLine[1], addressLines[1], "Address line 2 should match"); is(paymentAddress.country, storageAddress.country, "Country should match"); - is(paymentAddress.region, storageAddress["address-level1"], "Region should match"); + is(paymentAddress.region, storageAddress["address-level1"] || "", "Region should match"); is(paymentAddress.city, storageAddress["address-level2"], "City should match"); is(paymentAddress.postalCode, storageAddress["postal-code"], "Zip code should match"); is(paymentAddress.organization, storageAddress.organization, "Org should match"); @@ -266,6 +266,26 @@ async function setupPaymentDialog(browser, {methodData, details, options, mercha element.getBoundingClientRect().height; content.isHidden = (element) => elementHeight(element) == 0; content.isVisible = (element) => elementHeight(element) > 0; + content.fillField = async function fillField(field, value) { + // Keep in-sync with the copy in payments_common.js but with EventUtils methods called on a + // EventUtils object. + field.focus(); + if (field.localName == "select") { + if (field.value == value) { + // Do nothing + return; + } + field.value = value; + field.dispatchEvent(new content.window.Event("input")); + field.dispatchEvent(new content.window.Event("change")); + return; + } + while (field.value) { + EventUtils.sendKey("BACK_SPACE", content.window); + } + EventUtils.sendString(value, content.window); + } +; }); await injectEventUtilsInContentTask(frame); info("helper functions injected into frame"); @@ -352,7 +372,7 @@ async function selectPaymentDialogShippingAddressByCountry(frame, country) { } async function navigateToAddAddressPage(frame, aOptions = { - addLinkSelector: "address-picker a.add-link", + addLinkSelector: "address-picker[selected-state-key=\"selectedShippingAddress\"] a.add-link", initialPageId: "payment-summary", }) { await spawnPaymentDialogTask(frame, async (options) => { @@ -378,24 +398,59 @@ async function navigateToAddAddressPage(frame, aOptions = { }, aOptions); } +async function fillInBillingAddressForm(frame, aAddress) { + // For now billing and shipping address forms have the same fields but that may + // change so use separarate helpers. + return fillInShippingAddressForm(frame, aAddress); +} + +async function fillInShippingAddressForm(frame, aAddress, aOptions) { + let address = Object.assign({}, aAddress); + // Email isn't used on address forms, only payer/contact ones. + delete address.email; + return fillInAddressForm(frame, address, aOptions); +} + +async function fillInPayerAddressForm(frame, aAddress) { + let address = Object.assign({}, aAddress); + let payerFields = ["given-name", "additional-name", "family-name", "tel", "email"]; + for (let fieldName of Object.keys(address)) { + if (payerFields.includes(fieldName)) { + continue; + } + delete address[fieldName]; + } + return fillInAddressForm(frame, address); +} + async function fillInAddressForm(frame, aAddress, aOptions = {}) { await spawnPaymentDialogTask(frame, async (args) => { let {address, options = {}} = args; + if (typeof(address.country) != "undefined") { + // Set the country first so that the appropriate fields are visible. + let countryField = content.document.getElementById("country"); + ok(!countryField.disabled, "Country Field shouldn't be disabled"); + await content.fillField(countryField, address.country); + is(countryField.value, address.country, "country value is correct after fillField"); + } + // fill the form - info("manuallyAddAddress: fill the form with address: " + JSON.stringify(address)); + info("fillInAddressForm: fill the form with address: " + JSON.stringify(address)); for (let [key, val] of Object.entries(address)) { let field = content.document.getElementById(key); if (!field) { ok(false, `${key} field not found`); } - field.value = val; + ok(!field.disabled, `Field #${key} shouldn't be disabled`); + await content.fillField(field, val); + is(field.value, val, `${key} value is correct after fillField`); } let persistCheckbox = Cu.waiveXrays( - content.document.querySelector(options.checkboxSelector)); + content.document.querySelector("#address-page .persist-checkbox")); // only touch the checked state if explicitly told to in the options if (options.hasOwnProperty("setPersistCheckedValue")) { - info("fillInCardForm: Manually setting the persist checkbox checkedness to: " + + info("fillInAddressForm: Manually setting the persist checkbox checkedness to: " + options.setPersistCheckedValue); Cu.waiveXrays(persistCheckbox).checked = options.setPersistCheckedValue; } @@ -455,7 +510,7 @@ async function submitAddressForm(frame, aAddress, aOptions = {}) { }, {address: aAddress, options: aOptions}); } -async function manuallyAddAddress(frame, aAddress, aOptions = {}) { +async function manuallyAddShippingAddress(frame, aAddress, aOptions = {}) { let options = Object.assign({ expectPersist: true, isEditing: false, @@ -463,9 +518,10 @@ async function manuallyAddAddress(frame, aAddress, aOptions = {}) { checkboxSelector: "#address-page .persist-checkbox", }); await navigateToAddAddressPage(frame); - info("manuallyAddAddress, fill in address form with options: " + JSON.stringify(options)); - await fillInAddressForm(frame, aAddress, options); - info("manuallyAddAddress, verifyPersistCheckbox with options: " + JSON.stringify(options)); + info("manuallyAddShippingAddress, fill in address form with options: " + JSON.stringify(options)); + await fillInShippingAddressForm(frame, aAddress, options); + info("manuallyAddShippingAddress, verifyPersistCheckbox with options: " + + JSON.stringify(options)); await verifyPersistCheckbox(frame, options); await submitAddressForm(frame, aAddress, options); } diff --git a/browser/components/payments/test/mochitest/payments_common.js b/browser/components/payments/test/mochitest/payments_common.js index 770b2d55d388..38b1fb88267c 100644 --- a/browser/components/payments/test/mochitest/payments_common.js +++ b/browser/components/payments/test/mochitest/payments_common.js @@ -1,7 +1,7 @@ "use strict"; /* exported asyncElementRendered, promiseStateChange, promiseContentToChromeMessage, deepClone, - PTU, registerConsoleFilter */ + PTU, registerConsoleFilter, fillField */ const PTU = SpecialPowers.Cu.import("resource://testing-common/PaymentTestUtils.jsm", {}) .PaymentTestUtils; @@ -47,6 +47,30 @@ function deepClone(obj) { return JSON.parse(JSON.stringify(obj)); } +/** + * @param {HTMLElement} field + * @param {string} value + * @note This is async in case we need to make it async to handle focus in the future. + * @note Keep in sync with the copy in head.js + */ +async function fillField(field, value) { + field.focus(); + if (field.localName == "select") { + if (field.value == value) { + // Do nothing + return; + } + field.value = value; + field.dispatchEvent(new Event("input")); + field.dispatchEvent(new Event("change")); + return; + } + while (field.value) { + sendKey("BACK_SPACE"); + } + sendString(value); +} + /** * If filterFunction is a function which returns true given a console message * then the test won't fail from that message. diff --git a/browser/components/payments/test/mochitest/test_address_form.html b/browser/components/payments/test/mochitest/test_address_form.html index 01df174befe3..ae636837fff5 100644 --- a/browser/components/payments/test/mochitest/test_address_form.html +++ b/browser/components/payments/test/mochitest/test_address_form.html @@ -59,11 +59,7 @@ function checkAddressForm(customEl, expectedAddress) { } function sendStringAndCheckValidity(element, string, isValid) { - element.focus(); - while (element.value) { - sendKey("BACK_SPACE"); - } - sendString(string); + fillField(element, string); ok(element.checkValidity() == isValid, `${element.id} should be ${isValid ? "valid" : "invalid"} (${string})`); } @@ -115,25 +111,26 @@ add_task(async function test_saveButton() { display.appendChild(form); await asyncElementRendered(); - form.form.querySelector("#given-name").focus(); - sendString("Jaws"); - form.form.querySelector("#family-name").focus(); - sendString("Swaj"); - form.form.querySelector("#organization").focus(); - sendString("Allizom"); - form.form.querySelector("#street-address").focus(); - sendString("404 Internet Super Highway"); - form.form.querySelector("#address-level2").focus(); - sendString("Firefoxity City"); - form.form.querySelector("#address-level1").focus(); - sendString("CA"); - form.form.querySelector("#postal-code").focus(); - sendString("00001"); - form.form.querySelector("#country option[value='US']").selected = true; - form.form.querySelector("#email").focus(); - sendString("test@example.com"); - form.form.querySelector("#tel").focus(); - sendString("+15555551212"); + ok(form.saveButton.disabled, "Save button initially disabled"); + fillField(form.form.querySelector("#given-name"), "Jaws"); + fillField(form.form.querySelector("#family-name"), "Swaj"); + fillField(form.form.querySelector("#organization"), "Allizom"); + fillField(form.form.querySelector("#street-address"), "404 Internet Super Highway"); + fillField(form.form.querySelector("#address-level2"), "Firefoxity City"); + fillField(form.form.querySelector("#address-level1"), "CA"); + fillField(form.form.querySelector("#postal-code"), "00001"); + fillField(form.form.querySelector("#country"), "US"); + 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"); + + fillField(form.form.querySelector("#street-address"), "404 Internet Super Highway"); + ok(!form.saveButton.disabled, "Save button is enabled after re-filling street-address"); let messagePromise = promiseContentToChromeMessage("updateAutofillRecord"); is(form.saveButton.textContent, "Add", "Check label"); @@ -158,7 +155,6 @@ add_task(async function test_saveButton() { "address-level1": "CA", "postal-code": "00001", "country": "US", - "email": "test@example.com", "tel": "+15555551212", }, }, "Check event details for the message to chrome"); @@ -206,6 +202,8 @@ add_task(async function test_edit() { await asyncElementRendered(); 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"], @@ -225,6 +223,7 @@ add_task(async function test_edit() { 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"); info("change to no selected address"); await form.requestStore.setState({ @@ -235,6 +234,7 @@ add_task(async function test_edit() { }); await asyncElementRendered(); checkAddressForm(form, {}); + ok(form.saveButton.disabled, "Save button should be disabled for an empty form"); form.remove(); }); @@ -245,8 +245,14 @@ add_task(async function test_restricted_address_fields() { form.dataset.errorGenericSave = "Generic error"; await form.promiseReady; display.appendChild(form); + await form.requestStore.setState({ + "address-page": { + addressFields: "name email tel", + }, + }); await asyncElementRendered(); - form.setAttribute("address-fields", "name email tel"); + + ok(form.saveButton.disabled, "Save button should be disabled due to empty fields"); ok(!isHidden(form.form.querySelector("#given-name")), "given-name should be visible"); @@ -271,7 +277,18 @@ add_task(async function test_restricted_address_fields() { ok(!isHidden(form.form.querySelector("#tel")), "tel should be visible"); + fillField(form.form.querySelector("#given-name"), "John"); + 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"); + fillField(form.form.querySelector("#tel"), "+15555555555"); + ok(!form.saveButton.disabled, "Save button should be enabled with all required fields filled"); + form.remove(); + await form.requestStore.setState({ + "address-page": {}, + }); }); add_task(async function test_field_validation() { @@ -298,8 +315,8 @@ add_task(async function test_field_validation() { countrySelect, ]; for (let field of requiredFields) { - let container = field.closest("label"); - ok(container.hasAttribute("required"), "Container should have required attribute"); + let container = field.closest(`#${field.id}-container`); + ok(container.hasAttribute("required"), `#${field.id} container should have required attribute`); let span = container.querySelector("span"); is(span.getAttribute("fieldRequiredSymbol"), "*", "span should have asterisk as fieldRequiredSymbol"); @@ -307,8 +324,9 @@ add_task(async function test_field_validation() { "Asterisk should be on " + field.id); } - countrySelect.selectedIndex = [...countrySelect.options].findIndex(o => o.value == "US"); - countrySelect.dispatchEvent(new Event("change")); + ok(form.saveButton.disabled, "Save button should be disabled upon load"); + + fillField(countrySelect, "US"); sendStringAndCheckValidity(addressLevel1Input, "MI", true); sendStringAndCheckValidity(addressLevel1Input, "", false); @@ -320,8 +338,7 @@ add_task(async function test_field_validation() { sendStringAndCheckValidity(addressLevel1Input, "Nova Scotia", true); sendStringAndCheckValidity(postalCodeInput, "06390-0001", true); - countrySelect.selectedIndex = [...countrySelect.options].findIndex(o => o.value == "CA"); - countrySelect.dispatchEvent(new Event("change")); + fillField(countrySelect, "CA"); sendStringAndCheckValidity(postalCodeInput, "00001", false); sendStringAndCheckValidity(addressLevel1Input, "CA", true); @@ -373,6 +390,8 @@ add_task(async function test_customValidity() { "Validation message should match for " + selector); } + ok(form.saveButton.disabled, "Save button should be disabled due to validation errors"); + checkValidationMessage("#street-address", "addressLine"); checkValidationMessage("#address-level2", "city"); checkValidationMessage("#country", "country"); @@ -382,6 +401,8 @@ add_task(async function test_customValidity() { checkValidationMessage("#given-name", "recipient"); checkValidationMessage("#address-level1", "region"); + // TODO: bug 1482808 - the save button should be enabled after editing the fields + form.remove(); }); @@ -416,6 +437,8 @@ add_task(async function test_field_validation() { display.appendChild(form); await asyncElementRendered(); + ok(form.saveButton.disabled, "Save button should be disabled due to empty fields"); + let postalCodeInput = form.form.querySelector("#postal-code"); let addressLevel1Input = form.form.querySelector("#address-level1"); ok(!postalCodeInput.value, "postal-code should be empty by default"); diff --git a/browser/extensions/formautofill/content/autofillEditForms.js b/browser/extensions/formautofill/content/autofillEditForms.js index 834c1183405d..a1abf1879301 100644 --- a/browser/extensions/formautofill/content/autofillEditForms.js +++ b/browser/extensions/formautofill/content/autofillEditForms.js @@ -113,20 +113,72 @@ class EditAddress extends EditAutofillForm { this.formatForm(record.country); } + /** + * `mailing-address` is a special attribute token to indicate mailing fields + country. + * + * @param {object[]} mailingFieldsOrder - `fieldsOrder` from `getFormFormat` + * @returns {object[]} in the same structure as `mailingFieldsOrder` but including non-mail fields + */ + computeVisibleFields(mailingFieldsOrder) { + let addressFields = this._elements.form.dataset.addressFields; + if (addressFields) { + let requestedFieldClasses = addressFields.trim().split(/\s+/); + let fieldClasses = []; + if (requestedFieldClasses.includes("mailing-address")) { + fieldClasses = fieldClasses.concat(mailingFieldsOrder); + // `country` isn't part of the `mailingFieldsOrder` so add it when filling a mailing-address + requestedFieldClasses.splice(requestedFieldClasses.indexOf("mailing-address"), 1, + "country"); + } + + for (let fieldClassName of requestedFieldClasses) { + fieldClasses.push({ + fieldId: fieldClassName, + newLine: fieldClassName == "name", + }); + } + return fieldClasses; + } + + // This is the default which is shown in the management interface and includes all fields. + return mailingFieldsOrder.concat([ + { + fieldId: "country", + }, + { + fieldId: "tel", + }, + { + fieldId: "email", + newLine: true, + }, + ]); + } + /** * Format the form based on country. The address-level1 and postal-code labels * should be specific to the given country. * @param {string} country */ formatForm(country) { - const {addressLevel1Label, postalCodeLabel, fieldsOrder, postalCodePattern} = - this.getFormFormat(country); + const { + addressLevel1Label, + postalCodeLabel, + fieldsOrder: mailingFieldsOrder, + postalCodePattern, + } = this.getFormFormat(country); this._elements.addressLevel1Label.dataset.localization = addressLevel1Label; this._elements.postalCodeLabel.dataset.localization = postalCodeLabel; - this.arrangeFields(fieldsOrder); + let fieldClasses = this.computeVisibleFields(mailingFieldsOrder); + this.arrangeFields(fieldClasses); this.updatePostalCodeValidation(postalCodePattern); } + /** + * Update address field visibility and order based on libaddressinput data. + * + * @param {object[]} fieldsOrder array of objects with `fieldId` and optional `newLine` properties + */ arrangeFields(fieldsOrder) { let fields = [ "name", @@ -135,6 +187,9 @@ class EditAddress extends EditAutofillForm { "address-level2", "address-level1", "postal-code", + "country", + "tel", + "email", ]; let inputs = []; for (let i = 0; i < fieldsOrder.length; i++) { diff --git a/browser/extensions/formautofill/content/editAddress.xhtml b/browser/extensions/formautofill/content/editAddress.xhtml index 8ab2b2e8f832..26196d712528 100644 --- a/browser/extensions/formautofill/content/editAddress.xhtml +++ b/browser/extensions/formautofill/content/editAddress.xhtml @@ -22,7 +22,7 @@