Bug 1490805 - Add a required CSC/CVV field to the add card page. r=jaws
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 27 Sep 2018 17:26:24 +0000
changeset 438578 4ff0fcd61c7a1f791414516a6e22efca94530f19
parent 438577 39762ef5d56e07f8046bf9dfb1b32f194c6ea1bd
child 438579 50d1b7c5b1a170997c7706e8f8fc5ff4df730077
push id70044
push usermozilla@noorenberghe.ca
push dateThu, 27 Sep 2018 17:27:49 +0000
treeherderautoland@4ff0fcd61c7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1490805
milestone64.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 1490805 - Add a required CSC/CVV field to the add card page. r=jaws Depends on D6882 Differential Revision: https://phabricator.services.mozilla.com/D6883
browser/components/payments/content/paymentDialogWrapper.js
browser/components/payments/res/containers/address-form.js
browser/components/payments/res/containers/basic-card-form.js
browser/components/payments/res/containers/payment-method-picker.js
browser/components/payments/res/mixins/PaymentStateSubscriberMixin.js
browser/components/payments/res/paymentRequest.xhtml
browser/components/payments/test/browser/browser_card_edit.js
browser/components/payments/test/browser/browser_payments_onboarding_wizard.js
browser/components/payments/test/mochitest/test_basic_card_form.html
browser/extensions/formautofill/content/editCreditCard.xhtml
browser/extensions/formautofill/locales/en-US/formautofill.properties
browser/extensions/formautofill/skin/shared/editCreditCard.css
--- a/browser/components/payments/content/paymentDialogWrapper.js
+++ b/browser/components/payments/content/paymentDialogWrapper.js
@@ -123,17 +123,17 @@ var paymentDialogWrapper = {
   /**
    * @param {string} guid
    * @returns {nsIPaymentAddress}
    */
   async _convertProfileAddressToPaymentAddress(guid) {
     let addressData = this.temporaryStore.addresses.get(guid) ||
                       await formAutofillStorage.addresses.get(guid);
     if (!addressData) {
-      throw new Error(`Shipping address not found: ${guid}`);
+      throw new Error(`Address not found: ${guid}`);
     }
 
     let address = this.createPaymentAddress({
       country: addressData.country,
       addressLines: addressData["street-address"].split("\n"),
       region: addressData["address-level1"],
       city: addressData["address-level2"],
       dependentLocality: addressData["address-level3"],
--- a/browser/components/payments/res/containers/address-form.js
+++ b/browser/components/payments/res/containers/address-form.js
@@ -308,16 +308,17 @@ export default class AddressForm extends
       record.isTemporary = true;
     }
 
     let successStateChange;
     const previousId = page.previousId;
     if (page.onboardingWizard && !Object.keys(savedBasicCards).length) {
       successStateChange = {
         "basic-card-page": {
+          selectedStateKey: "selectedPaymentCard",
           // Preserve field values as the user may have already edited the card
           // page and went back to the address page to make a correction.
           preserveFieldValues: true,
         },
         page: {
           id: "basic-card-page",
           previousId: "address-page",
           onboardingWizard: page.onboardingWizard,
--- a/browser/components/payments/res/containers/basic-card-form.js
+++ b/browser/components/payments/res/containers/basic-card-form.js
@@ -164,16 +164,20 @@ export default class BasicCardForm exten
     let record = {};
     let basicCards = paymentRequest.getBasicCards(state);
     let addresses = paymentRequest.getAddresses(state);
 
     this.genericErrorText.textContent = page.error;
 
     this.form.querySelector("#cc-number").disabled = editing;
 
+    // The CVV fields should be hidden and disabled when editing.
+    this.form.querySelector("#cc-csc-container").hidden = editing;
+    this.form.querySelector("#cc-csc").disabled = editing;
+
     // If a card is selected we want to edit it.
     if (editing) {
       this.pageTitleHeading.textContent = this.dataset.editBasicCardTitle;
       record = basicCards[basicCardPage.guid];
       if (!record) {
         throw new Error("Trying to edit a non-existing card: " + basicCardPage.guid);
       }
       // When editing an existing record, prevent changes to persistence
@@ -279,16 +283,17 @@ export default class BasicCardForm exten
             guid: null,
             selectedStateKey: ["basic-card-page", "billingAddressGUID"],
             title: this.dataset.billingAddressTitleAdd,
           },
           "basic-card-page": {
             preserveFieldValues: true,
             guid: basicCardPage.guid,
             persistCheckboxValue: this.persistCheckbox.checked,
+            selectedStateKey: basicCardPage.selectedStateKey,
           },
         };
         let billingAddressGUID = this.form.querySelector("#billingAddressGUID");
         let selectedOption = billingAddressGUID.selectedOptions.length &&
                              billingAddressGUID.selectedOptions[0];
         if (evt.target == this.addressEditLink && selectedOption && selectedOption.value) {
           nextState["address-page"].title = this.dataset.billingAddressTitleEdit;
           nextState["address-page"].guid = selectedOption.value;
@@ -395,34 +400,43 @@ export default class BasicCardForm exten
       "basic-card-page": basicCardPage,
     } = currentState;
     let editing = !!basicCardPage.guid;
 
     if (editing ? (basicCardPage.guid in tempBasicCards) : !this.persistCheckbox.checked) {
       record.isTemporary = true;
     }
 
-    for (let editableFieldName of ["cc-name", "cc-exp-month", "cc-exp-year"]) {
+    for (let editableFieldName of ["cc-name", "cc-exp-month", "cc-exp-year", "cc-type"]) {
       record[editableFieldName] = record[editableFieldName] || "";
     }
 
     // Only save the card number if we're saving a new record, otherwise we'd
     // overwrite the unmasked card number with the masked one.
     if (!editing) {
       record["cc-number"] = record["cc-number"] || "";
     }
 
+    // Never save the CSC in storage. Storage will throw and not save the record
+    // if it is passed.
+    delete record["cc-csc"];
+
     try {
       let {guid} = await paymentRequest.updateAutofillRecord("creditCards", record,
                                                              basicCardPage.guid);
+      let {selectedStateKey} = currentState["basic-card-page"];
+      if (!selectedStateKey) {
+        throw new Error(`state["basic-card-page"].selectedStateKey is required`);
+      }
       this.requestStore.setState({
         page: {
           id: "payment-summary",
         },
-        selectedPaymentCard: guid,
+        [selectedStateKey]: guid,
+        [selectedStateKey + "SecurityCode"]: this.form.querySelector("#cc-csc").value,
       });
     } catch (ex) {
       log.warn("saveRecord: error:", ex);
       this.requestStore.setState({
         page: {
           id: "basic-card-page",
           error: this.dataset.errorGenericSave,
         },
--- a/browser/components/payments/res/containers/payment-method-picker.js
+++ b/browser/components/payments/res/containers/payment-method-picker.js
@@ -70,16 +70,21 @@ export default class PaymentMethodPicker
     let selectedPaymentCardGUID = state[this.selectedStateKey];
     this.dropdown.value = selectedPaymentCardGUID;
 
     if (selectedPaymentCardGUID && selectedPaymentCardGUID !== this.dropdown.value) {
       throw new Error(`The option ${selectedPaymentCardGUID} ` +
                       `does not exist in the payment method picker`);
     }
 
+    let securityCodeState = state[this.selectedStateKey + "SecurityCode"];
+    if (securityCodeState && securityCodeState != this.securityCodeInput.value) {
+      this.securityCodeInput.defaultValue = securityCodeState;
+    }
+
     super.render(state);
   }
 
   isSelectedOptionValid(state) {
     let hasMissingFields = this.missingFieldsOfSelectedOption().length;
     if (hasMissingFields) {
       return false;
     }
@@ -138,17 +143,19 @@ export default class PaymentMethodPicker
     this.requestStore.setState(stateChange);
   }
 
   onClick({target}) {
     let nextState = {
       page: {
         id: "basic-card-page",
       },
-      "basic-card-page": {},
+      "basic-card-page": {
+        selectedStateKey: this.selectedStateKey,
+      },
     };
 
     switch (target) {
       case this.addLink: {
         nextState["basic-card-page"].guid = null;
         break;
       }
       case this.editLink: {
--- a/browser/components/payments/res/mixins/PaymentStateSubscriberMixin.js
+++ b/browser/components/payments/res/mixins/PaymentStateSubscriberMixin.js
@@ -12,16 +12,17 @@ import PaymentsStore from "../PaymentsSt
  * State of the payment request dialog.
  */
 export let requestStore = new PaymentsStore({
   changesPrevented: false,
   orderDetailsShowing: false,
   "basic-card-page": {
     guid: null,
     // preserveFieldValues: true,
+    selectedStateKey: null,
   },
   "address-page": {
     guid: null,
     selectedStateKey: null,
     title: "",
   },
   "payment-summary": {
   },
--- a/browser/components/payments/res/paymentRequest.xhtml
+++ b/browser/components/payments/res/paymentRequest.xhtml
@@ -55,17 +55,17 @@
   <!ENTITY orderTotalLabel            "Total">
   <!ENTITY basicCardPage.error.genericSave    "There was an error saving the payment card.">
   <!ENTITY basicCardPage.addressAddLink.label "Add">
   <!ENTITY basicCardPage.addressEditLink.label "Edit">
   <!ENTITY basicCardPage.backButton.label     "Back">
   <!ENTITY basicCardPage.addButton.label      "Add">
   <!ENTITY basicCardPage.nextButton.label     "Next">
   <!ENTITY basicCardPage.updateButton.label   "Update">
-  <!ENTITY basicCardPage.persistCheckbox.label     "Save credit card to &brandShortName; (Security code will not be saved)">
+  <!ENTITY basicCardPage.persistCheckbox.label     "Save credit card to &brandShortName; (CVV will not be saved)">
   <!ENTITY basicCardPage.persistCheckbox.infoTooltip  "&brandShortName; can securely store your credit card information to use in forms like this, so you don’t have to enter it every time.">
   <!ENTITY addressPage.error.genericSave      "There was an error saving the address.">
   <!ENTITY addressPage.cancelButton.label     "Cancel">
   <!ENTITY addressPage.backButton.label       "Back">
   <!ENTITY addressPage.addButton.label        "Add">
   <!ENTITY addressPage.nextButton.label       "Next">
   <!ENTITY addressPage.updateButton.label     "Update">
   <!ENTITY addressPage.persistCheckbox.label  "Save address to &brandShortName;">
--- a/browser/components/payments/test/browser/browser_card_edit.js
+++ b/browser/components/payments/test/browser/browser_card_edit.js
@@ -56,17 +56,20 @@ async function add_link(aOptions = {}) {
       checkboxSelector: "basic-card-form .persist-checkbox",
       expectPersist: aOptions.expectCardPersist,
       networkSelector: "basic-card-form #cc-type",
       expectedNetwork: PTU.BasicCards.JaneMasterCard["cc-type"],
     });
     if (aOptions.hasOwnProperty("setCardPersistCheckedValue")) {
       cardOptions.setPersistCheckedValue = aOptions.setCardPersistCheckedValue;
     }
-    await fillInCardForm(frame, PTU.BasicCards.JaneMasterCard, cardOptions);
+    await fillInCardForm(frame, {
+      ["cc-csc"]: 123,
+      ...PTU.BasicCards.JaneMasterCard,
+    }, cardOptions);
 
     await verifyCardNetwork(frame, cardOptions);
     await verifyPersistCheckbox(frame, cardOptions);
 
     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");
@@ -645,17 +648,20 @@ add_task(async function test_private_car
       addLink.click();
 
       await PTU.DialogContentUtils.waitForState(content, (state) => {
         return state.page.id == "basic-card-page" && !state["basic-card-page"].guid;
       },
                                                 "Check card page state");
     });
 
-    await fillInCardForm(frame, PTU.BasicCards.JohnDoe);
+    await fillInCardForm(frame, {
+      ["cc-csc"]: "999",
+      ...PTU.BasicCards.JohnDoe,
+    });
 
     await spawnPaymentDialogTask(frame, async function() {
       let {
         PaymentTestUtils: PTU,
       } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
 
       let card = Object.assign({}, PTU.BasicCards.JohnDoe);
       let state = await PTU.DialogContentUtils.getCurrentState(content);
--- a/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js
+++ b/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js
@@ -86,17 +86,20 @@ add_task(async function test_onboarding_
 
       info("Check if the correct billing address is selected in the basic card page");
       PTU.DialogContentUtils.waitForState(content, (state) => {
         let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
         return state.selectedShippingAddress == billingAddressSelect.value;
       }, "Shipping address is selected as the billing address");
     });
 
-    await fillInCardForm(frame, PTU.BasicCards.JohnDoe);
+    await fillInCardForm(frame, {
+      ["cc-csc"]: "123",
+      ...PTU.BasicCards.JohnDoe,
+    });
 
     await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton);
 
     await spawnPaymentDialogTask(frame, async function() {
       let {
         PaymentTestUtils: PTU,
       } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
 
@@ -357,17 +360,20 @@ add_task(async function test_onboarding_
 
       info("Check if the correct billing address is selected in the basic card page");
       PTU.DialogContentUtils.waitForState(content, (state) => {
         let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
         return state["basic-card-page"].billingAddressGUID == billingAddressSelect.value;
       }, "Billing Address is correctly shown");
     });
 
-    await fillInCardForm(frame, PTU.BasicCards.JohnDoe);
+    await fillInCardForm(frame, {
+      ["cc-csc"]: "123",
+      ...PTU.BasicCards.JohnDoe,
+    });
 
     await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.clickPrimaryButton);
 
     await spawnPaymentDialogTask(frame, async function() {
       let {
         PaymentTestUtils: PTU,
       } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
 
--- a/browser/components/payments/test/mochitest/test_basic_card_form.html
+++ b/browser/components/payments/test/mochitest/test_basic_card_form.html
@@ -133,16 +133,17 @@ add_task(async function test_saveButton(
   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");
   fillField(form.form.querySelector("#cc-exp-month"), "11");
   let year = (new Date()).getFullYear().toString();
   fillField(form.form.querySelector("#cc-exp-year"), year);
   fillField(form.form.querySelector("#cc-type"), "visa");
+  fillField(form.form.querySelector("#cc-csc"), "123");
   isnot(form.form.querySelector("#billingAddressGUID").value, address2.guid,
         "Check initial billing address");
   fillField(form.form.querySelector("#billingAddressGUID"), address2.guid);
   is(form.form.querySelector("#billingAddressGUID").value, address2.guid,
      "Check selected billing address");
   form.saveButton.focus();
   ok(!form.saveButton.disabled,
      "Save button should be enabled since the required fields are filled");
@@ -198,17 +199,17 @@ add_task(async function test_saveButton(
 
 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);
-  is(requiredElements.length, 6, "Number of required elements");
+  is(requiredElements.length, 7, "Number of required elements");
   for (let element of requiredElements) {
     if (element.id == "billingAddressGUID") {
       // The billing address has a different layout.
       continue;
     }
     let container = element.closest("label") || element.closest("div");
     ok(container.hasAttribute("required"), "Container should also be marked as required");
   }
@@ -440,31 +441,33 @@ add_task(async function test_field_valid
   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");
   let typeInput = form.form.querySelector("#cc-type");
+  let cscInput = form.form.querySelector("#cc-csc");
   let monthInput = form.form.querySelector("#cc-exp-month");
   let yearInput = form.form.querySelector("#cc-exp-year");
 
   info("test with valid cc-number but missing cc-name");
   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 and expiration values");
   fillField(nameInput, "First");
   fillField(monthInput, "11");
   let year = (new Date()).getFullYear().toString();
   fillField(yearInput, year);
   fillField(typeInput, "visa");
+  fillField(cscInput, "456");
   ok(ccNumber.checkValidity(), "cc-number field is valid with good input");
   ok(nameInput.checkValidity(), "cc-name field is valid with a value");
   ok(monthInput.checkValidity(), "cc-exp-month field is valid with a value");
   ok(yearInput.checkValidity(), "cc-exp-year field is valid with a value");
   ok(typeInput.checkValidity(), "cc-type 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");
--- a/browser/extensions/formautofill/content/editCreditCard.xhtml
+++ b/browser/extensions/formautofill/content/editCreditCard.xhtml
@@ -55,16 +55,27 @@
       <input id="cc-name" type="text" required="required"/>
       <span data-localization="nameOnCard" class="label-text"/>
     </label>
     <label id="cc-type-container" class="container">
       <select id="cc-type" required="required">
       </select>
       <span data-localization="cardNetwork" class="label-text"/>
     </label>
+    <label id="cc-csc-container" class="container" hidden="hidden">
+      <!-- Keep these attributes in-sync with securityCodeInput in payment-method-picker.js -->
+      <input id="cc-csc"
+             type="text"
+             autocomplete="off"
+             size="3"
+             required="required"
+             pattern="[0-9]{3,}"
+             disabled="disabled"/>
+      <span data-localization="cardCVV" class="label-text"/>
+    </label>
     <div id="billingAddressGUID-container" class="billingAddressRow container rich-picker">
       <select id="billingAddressGUID" required="required">
       </select>
       <label for="billingAddressGUID" data-localization="billingAddress" class="label-text"/>
     </div>
   </form>
   <div id="controls-container">
     <button id="cancel" data-localization="cancelBtnLabel"/>
--- a/browser/extensions/formautofill/locales/en-US/formautofill.properties
+++ b/browser/extensions/formautofill/locales/en-US/formautofill.properties
@@ -180,16 +180,18 @@ addNewCreditCardTitle = Add New Credit C
 editCreditCardTitle = Edit Credit Card
 cardNumber = Card Number
 invalidCardNumber = Please enter a valid card number
 nameOnCard = Name on Card
 cardExpiresMonth = Exp. Month
 cardExpiresYear = Exp. Year
 billingAddress = Billing Address
 cardNetwork = Card Type
+# LOCALIZATION NOTE (cardCVV): Credit card security code https://en.wikipedia.org/wiki/Card_security_code
+cardCVV = CVV
 
 # LOCALIZATION NOTE: (cardNetwork.*): These are brand names and should only be translated when a locale-specific name for that brand is in common use
 cardNetwork.amex = American Express
 cardNetwork.cartebancaire = Carte Bancaire
 cardNetwork.diners = Diners Club
 cardNetwork.discover = Discover
 cardNetwork.jcb = JCB
 cardNetwork.mastercard = MasterCard
--- a/browser/extensions/formautofill/skin/shared/editCreditCard.css
+++ b/browser/extensions/formautofill/skin/shared/editCreditCard.css
@@ -39,15 +39,19 @@
 #cc-name-container {
   grid-area: cc-name;
 }
 
 #cc-type-container {
   grid-area: cc-type;
 }
 
+#cc-csc-container {
+  grid-area: cc-csc;
+}
+
 #billingAddressGUID-container {
   grid-area: billingAddressGUID;
 }
 
 #billingAddressGUID {
   grid-area: dropdown;
 }