Bug 1477114 - Add an asterisk to the required fields of the credit card form as well as the CVV placeholder. r=sfoster
authorJared Wein <jwein@mozilla.com>
Mon, 30 Jul 2018 17:52:24 -0400
changeset 429913 8b2d475dff2c22ed339565a1e4c8a38b0e307643
parent 429912 ade87af5847e5ca237895a9fda96fe0867ee3a60
child 429914 2698872fcdef1837566f3809f983bdb04205e3f9
push id34376
push userbtara@mozilla.com
push dateFri, 03 Aug 2018 10:10:33 +0000
treeherdermozilla-central@484dc9b59dca [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster
bugs1477114
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 1477114 - Add an asterisk to the required fields of the credit card form as well as the CVV placeholder. r=sfoster MozReview-Commit-ID: 2zg5HOZVtxN
browser/components/payments/res/containers/address-form.css
browser/components/payments/res/containers/address-form.js
browser/components/payments/res/containers/basic-card-form.js
browser/components/payments/res/containers/form.css
browser/components/payments/res/containers/payment-method-picker.js
browser/components/payments/res/paymentRequest.xhtml
browser/components/payments/test/mochitest/test_address_form.html
browser/components/payments/test/mochitest/test_basic_card_form.html
--- a/browser/components/payments/res/containers/address-form.css
+++ b/browser/components/payments/res/containers/address-form.css
@@ -19,21 +19,16 @@ address-form[address-fields] #postal-cod
 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;
 }
 
-label[required] > span:first-of-type::after {
-  /* The asterisk should be localized, bug 1472278 */
-  content: "*";
-}
-
 .error-text:not(:empty) {
   color: #fff;
   background-color: #d70022;
   border-radius: 2px;
   /* The padding-top and padding-bottom are referenced by address-form.js */
   padding: 5px 12px;
   position: absolute;
   z-index: 1;
--- a/browser/components/payments/res/containers/address-form.js
+++ b/browser/components/payments/res/containers/address-form.js
@@ -148,28 +148,17 @@ export default class AddressForm extends
       this.persistCheckbox.hidden = false;
       this.persistCheckbox.checked = state.isPrivate ? false :
                                                        defaults.saveAddressDefaultChecked;
     }
 
     this.formHandler.loadRecord(record);
 
     // Add validation to some address fields
-    for (let formElement of this.form.elements) {
-      let container = formElement.closest(`#${formElement.id}-container`);
-      if (formElement.localName == "button" || !container) {
-        continue;
-      }
-      let required = formElement.required && !formElement.disabled;
-      if (required) {
-        container.setAttribute("required", "true");
-      } else {
-        container.removeAttribute("required");
-      }
-    }
+    this.updateRequiredState();
 
     let shippingAddressErrors = request.paymentDetails.shippingAddressErrors;
     for (let [errorName, errorSelector] of Object.entries(this._errorFieldMap)) {
       let container = this.form.querySelector(errorSelector + "-container");
       let field = this.form.querySelector(errorSelector);
       let errorText = (shippingAddressErrors && shippingAddressErrors[errorName]) || "";
       container.classList.toggle("error", !!errorText);
       field.setCustomValidity(errorText);
@@ -200,16 +189,17 @@ export default class AddressForm extends
       data.span.style.top = (data.top - 10) + "px";
       if (isRTL) {
         data.span.style.right = data.right + "px";
       } else {
         data.span.style.left = data.left + "px";
       }
     }
   }
+
   handleEvent(event) {
     switch (event.type) {
       case "click": {
         this.onClick(event);
         break;
       }
     }
   }
@@ -241,16 +231,31 @@ export default class AddressForm extends
         break;
       }
       default: {
         throw new Error("Unexpected click target");
       }
     }
   }
 
+  updateRequiredState() {
+    for (let formElement of this.form.elements) {
+      let container = formElement.closest(`#${formElement.id}-container`);
+      if (formElement.localName == "button" || !container) {
+        continue;
+      }
+      let required = formElement.required && !formElement.disabled;
+      if (required) {
+        container.setAttribute("required", "true");
+      } else {
+        container.removeAttribute("required");
+      }
+    }
+  }
+
   async saveRecord() {
     let record = this.formHandler.buildFormObject();
     let currentState = this.requestStore.getState();
     let {
       page,
       tempAddresses,
       savedBasicCards,
       "address-page": addressPage,
--- a/browser/components/payments/res/containers/basic-card-form.js
+++ b/browser/components/payments/res/containers/basic-card-form.js
@@ -176,16 +176,17 @@ export default class BasicCardForm exten
     } else if (!editing) {
       if (paymentRequest.getAddresses(state)[selectedShippingAddress]) {
         billingAddressSelect.value = selectedShippingAddress;
       } else {
         billingAddressSelect.value = Object.keys(addresses)[0];
       }
     }
 
+    this.updateRequiredState();
     this.updateSaveButtonState();
   }
 
   handleEvent(event) {
     switch (event.type) {
       case "click": {
         this.onClick(event);
         break;
@@ -294,16 +295,28 @@ export default class BasicCardForm exten
   onInvalid(event) {
     this.saveButton.disabled = true;
   }
 
   updateSaveButtonState() {
     this.saveButton.disabled = !this.form.checkValidity();
   }
 
+  updateRequiredState() {
+    for (let formElement of this.form.elements) {
+      let container = formElement.closest("label") || formElement.closest("div");
+      let required = formElement.required && !formElement.disabled;
+      if (required) {
+        container.setAttribute("required", "true");
+      } else {
+        container.removeAttribute("required");
+      }
+    }
+  }
+
   async saveRecord() {
     let record = this.formHandler.buildFormObject();
     let currentState = this.requestStore.getState();
     let {
       tempBasicCards,
       "basic-card-page": basicCardPage,
     } = currentState;
     let editing = !!basicCardPage.guid;
new file mode 100644
--- /dev/null
+++ b/browser/components/payments/res/containers/form.css
@@ -0,0 +1,8 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+:-moz-any(label, div)[required] > span:first-of-type::after {
+  /* The asterisk should be localized, bug 1472278 */
+  content: "*";
+}
--- a/browser/components/payments/res/containers/payment-method-picker.js
+++ b/browser/components/payments/res/containers/payment-method-picker.js
@@ -13,17 +13,17 @@ import paymentRequest from "../paymentRe
  */
 
 export default class PaymentMethodPicker extends RichPicker {
   constructor() {
     super();
     this.dropdown.setAttribute("option-type", "basic-card-option");
     this.securityCodeInput = document.createElement("input");
     this.securityCodeInput.autocomplete = "off";
-    this.securityCodeInput.placeholder = "CVV"; /* XXX Bug 1473772 */
+    this.securityCodeInput.placeholder = "CVV*"; /* XXX Bug 1473772 */
     this.securityCodeInput.size = 3;
     this.securityCodeInput.classList.add("security-code");
     this.securityCodeInput.addEventListener("change", this);
   }
 
   connectedCallback() {
     super.connectedCallback();
     this.dropdown.after(this.securityCodeInput);
--- a/browser/components/payments/res/paymentRequest.xhtml
+++ b/browser/components/payments/res/paymentRequest.xhtml
@@ -77,16 +77,17 @@
 
   <link rel="stylesheet" href="chrome://global/skin/in-content/common.css"/>
   <link rel="stylesheet" href="paymentRequest.css"/>
   <link rel="stylesheet" href="components/rich-select.css"/>
   <link rel="stylesheet" href="components/address-option.css"/>
   <link rel="stylesheet" href="components/basic-card-option.css"/>
   <link rel="stylesheet" href="components/shipping-option.css"/>
   <link rel="stylesheet" href="components/payment-details-item.css"/>
+  <link rel="stylesheet" href="containers/form.css"/>
   <link rel="stylesheet" href="containers/address-form.css"/>
   <link rel="stylesheet" href="containers/basic-card-form.css"/>
   <link rel="stylesheet" href="containers/order-details.css"/>
   <link rel="stylesheet" href="containers/rich-picker.css"/>
   <link rel="stylesheet" href="containers/error-page.css"/>
 
   <script src="unprivileged-fallbacks.js"></script>
 
--- a/browser/components/payments/test/mochitest/test_address_form.html
+++ b/browser/components/payments/test/mochitest/test_address_form.html
@@ -12,16 +12,17 @@ Test the address-form element
   <script src="sinon-2.3.2.js"></script>
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
   <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
+  <link rel="stylesheet" type="text/css" href="../../res/containers/form.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/containers/address-form.css"/>
 </head>
 <body>
   <p id="display">
   </p>
 <div id="content" style="display: none">
 
 </div>
@@ -293,18 +294,18 @@ add_task(async function test_field_valid
     form.form.querySelector("#address-level2"),
     postalCodeInput,
     addressLevel1Input,
     countrySelect,
   ];
   for (let field of requiredFields) {
     let container = field.closest("label");
     ok(container.hasAttribute("required"), "Container should have required attribute");
-    let label = field.closest("label").querySelector("span");
-    is(getComputedStyle(label, "::after").content, "\"*\"", "Asterisk should be on " + field.id);
+    let span = container.querySelector("span");
+    is(getComputedStyle(span, "::after").content, "\"*\"", "Asterisk should be on " + field.id);
   }
 
   countrySelect.selectedIndex = [...countrySelect.options].findIndex(o => o.value == "US");
   countrySelect.dispatchEvent(new Event("change"));
 
   sendStringAndCheckValidity(addressLevel1Input, "MI", true);
   sendStringAndCheckValidity(addressLevel1Input, "", false);
   sendStringAndCheckValidity(postalCodeInput, "B4N4N4", false);
--- a/browser/components/payments/test/mochitest/test_basic_card_form.html
+++ b/browser/components/payments/test/mochitest/test_basic_card_form.html
@@ -131,16 +131,48 @@ add_task(async function test_saveButton(
       "cc-number": "4111 1111-1111 1111",
       "billingAddressGUID": "",
       "isTemporary": true,
     },
   }, "Check event details for the message to chrome");
   form.remove();
 });
 
+add_task(async function test_requiredAttributePropagated() {
+  let form = new BasicCardForm();
+  await form.promiseReady;
+  display.appendChild(form);
+  await asyncElementRendered();
+
+  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(container.hasAttribute("required"), "Container should also be marked as required");
+  }
+  // Now test that toggling the `required` attribute will affect the container.
+  let sampleRequiredElement = requiredElements[0];
+  let sampleRequiredContainer = sampleRequiredElement.closest("label") ||
+                                sampleRequiredElement.closest("div");
+  sampleRequiredElement.removeAttribute("required");
+  await form.requestStore.setState({});
+  await asyncElementRendered();
+  ok(!sampleRequiredElement.hasAttribute("required"),
+     `"required" attribute should still be removed from element (${sampleRequiredElement.id})`);
+  ok(!sampleRequiredContainer.hasAttribute("required"),
+     `"required" attribute should be removed from container`);
+  sampleRequiredElement.setAttribute("required", "true");
+  await form.requestStore.setState({});
+  await asyncElementRendered();
+  ok(sampleRequiredContainer.hasAttribute("required"),
+     "`required` attribute is re-added to container");
+
+  form.remove();
+});
+
 add_task(async function test_genericError() {
   let form = new BasicCardForm();
   await form.requestStore.setState({
     page: {
       id: "test-page",
       error: "Generic Error",
     },
   });
@@ -257,16 +289,24 @@ add_task(async function test_edit() {
     savedBasicCards: {
       [card1.guid]: deepClone(card1),
     },
   });
   await asyncElementRendered();
   is(form.saveButton.textContent, "Update", "Check label");
   checkCCForm(form, card1);
 
+  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");
+  }
+
   info("test future year");
   card1["cc-exp-year"] = 2100;
 
   await form.requestStore.setState({
     savedBasicCards: {
       [card1.guid]: deepClone(card1),
     },
   });