Bug 1428415 Add a checkbox for persisting new cards to the Add Payment Card screen. r=MattN

* Add a new labelled-checkbox component, and use it for the persist checkbox in basic card add/edit form
* Pass an isPrivate flag from the parent to UI in the state
* Re-work save logic for the basic card form to set correct defaults when payment is initiated from a private window
* Add a tempBasicCards object on the state, and a paymentRequest.getBasicCards(state) helper to get the union of both saved and temporary cards
* Set a newly added temporary card as the selectedPaymentCard
* Tests for basic-card-form.js in private windows, and correctly persisting or not new card info basic on the state of the 'Save to Firefox' checkbox
* Add paymentRequest.js to mochitests, pending landing of bug 1427939

MozReview-Commit-ID: 9oQ1gbHPojf

--HG--
extra : rebase_source : 947b74d62d9257d1ee27dbaffe47e9f907543518

toolkit/components/payments/content/paymentDialogWrapper.js

@@ -17,6 +17,8 @@ ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(this, "MasterPassword",
                                "resource://formautofill/MasterPassword.jsm");
+ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
+                               "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "formAutofillStorage", () => {
   let formAutofillStorage;
@@ -383,10 +385,14 @@ var paymentDialogWrapper = {
 
   initializeFrame() {
     let requestSerialized = this._serializeRequest(this.request);
+    let chromeWindow = Services.wm.getMostRecentWindow("navigator:browser");
+    let isPrivate = PrivateBrowsingUtils.isWindowPrivate(chromeWindow);
+
     this.sendMessageToContent("showPaymentRequest", {
       request: requestSerialized,
       savedAddresses: this.fetchSavedAddresses(),
       savedBasicCards: this.fetchSavedPaymentCards(),
+      isPrivate,
     });
 
     Services.obs.addObserver(this, "formautofill-storage-changed", true);

toolkit/components/payments/res/components/labelled-checkbox.js

@@ -0,0 +1,47 @@
+/* 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/. */
+
+import ObservedPropertiesMixin from "../mixins/ObservedPropertiesMixin.js";
+
+/**
+ *  <labelled-checkbox label="Some label" value="The value"></labelled-checkbox>
+ */
+
+export default class LabelledCheckbox extends ObservedPropertiesMixin(HTMLElement) {
+  static get observedAttributes() {
+    return [
+      "label",
+      "value",
+    ];
+  }
+  constructor() {
+    super();
+
+    this._label = document.createElement("label");
+    this._labelSpan = document.createElement("span");
+    this._checkbox = document.createElement("input");
+    this._checkbox.type = "checkbox";
+  }
+
+  connectedCallback() {
+    this.appendChild(this._label);
+    this._label.appendChild(this._checkbox);
+    this._label.appendChild(this._labelSpan);
+    this.render();
+  }
+
+  render() {
+    this._labelSpan.textContent = this.label;
+  }
+
+  get checked() {
+    return this._checkbox.checked;
+  }
+
+  set checked(value) {
+    return this._checkbox.checked = value;
+  }
+}
+
+customElements.define("labelled-checkbox", LabelledCheckbox);

toolkit/components/payments/res/containers/basic-card-form.js

@@ -3,8 +3,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* import-globals-from ../../../../../browser/extensions/formautofill/content/autofillEditForms.js*/
+import LabelledCheckbox from "../components/labelled-checkbox.js";
 import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
 import paymentRequest from "../paymentRequest.js";
+
 /* import-globals-from ../unprivileged-fallbacks.js */
 
 /**
@@ -26,6 +28,8 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
     this.saveButton = document.createElement("button");
     this.saveButton.addEventListener("click", this);
 
+    this.persistCheckbox = new LabelledCheckbox();
+
     // The markup is shared with form autofill preferences.
     let url = "formautofill/editCreditCard.xhtml";
     this.promiseReady = this._fetchMarkup(url).then(doc => {
@@ -60,6 +64,7 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
         getAddressLabel: PaymentDialogUtils.getAddressLabel,
       });
 
+      this.appendChild(this.persistCheckbox);
       this.appendChild(this.genericErrorText);
       this.appendChild(this.backButton);
       this.appendChild(this.saveButton);
@@ -72,14 +77,15 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
   render(state) {
     this.backButton.textContent = this.dataset.backButtonLabel;
     this.saveButton.textContent = this.dataset.saveButtonLabel;
+    this.persistCheckbox.label = this.dataset.persistCheckboxLabel;
 
     let record = {};
     let {
       page,
       savedAddresses,
-      savedBasicCards,
       selectedShippingAddress,
     } = state;
+    let basicCards = paymentRequest.getBasicCards(state);
 
     this.genericErrorText.textContent = page.error;
 
@@ -88,12 +94,19 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
 
     // If a card is selected we want to edit it.
     if (editing) {
-      record = savedBasicCards[page.guid];
+      record = basicCards[page.guid];
       if (!record) {
         throw new Error("Trying to edit a non-existing card: " + page.guid);
       }
-    } else if (selectedShippingAddress) {
-      record.billingAddressGUID = selectedShippingAddress;
+      // When editing an existing record, prevent changes to persistence
+      this.persistCheckbox.hidden = true;
+    } else {
+      if (selectedShippingAddress) {
+        record.billingAddressGUID = selectedShippingAddress;
+      }
+      // Adding a new record: default persistence to checked when in a not-private session
+      this.persistCheckbox.hidden = false;
+      this.persistCheckbox.checked = !state.isPrivate;
     }
 
     this.formHandler.loadRecord(record, savedAddresses);
@@ -132,7 +145,10 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
     let record = this.formHandler.buildFormObject();
     let {
       page,
+      tempBasicCards,
     } = this.requestStore.getState();
+    let editing = !!page.guid;
+    let tempRecord = editing && tempBasicCards[page.guid];
 
     for (let editableFieldName of ["cc-name", "cc-exp-month", "cc-exp-year"]) {
       record[editableFieldName] = record[editableFieldName] || "";
@@ -140,25 +156,44 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme
 
     // 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 (!page.guid) {
+    if (!editing) {
       record["cc-number"] = record["cc-number"] || "";
     }
 
-    paymentRequest.updateAutofillRecord("creditCards", record, page.guid, {
-      errorStateChange: {
-        page: {
-          id: "basic-card-page",
-          error: this.dataset.errorGenericSave,
+    if (!tempRecord && this.persistCheckbox.checked) {
+      log.debug(`BasicCardForm: persisting creditCard record: ${page.guid || "(new)"}`);
+      paymentRequest.updateAutofillRecord("creditCards", record, page.guid, {
+        errorStateChange: {
+          page: {
+            id: "basic-card-page",
+            error: this.dataset.errorGenericSave,
+          },
         },
-      },
-      preserveOldProperties: true,
-      selectedStateKey: "selectedPaymentCard",
-      successStateChange: {
+        preserveOldProperties: true,
+        selectedStateKey: "selectedPaymentCard",
+        successStateChange: {
+          page: {
+            id: "payment-summary",
+          },
+        },
+      });
+    } else {
+      // This record will never get inserted into the store
+      // so we generate a faux-guid for a new record
+      record.guid = page.guid || "temp-" + Math.abs(Math.random() * 0xffffffff|0);
+
+      log.debug(`BasicCardForm: saving temporary record: ${record.guid}`);
+      this.requestStore.setState({
         page: {
           id: "payment-summary",
         },
-      },
-    });
+        selectedPaymentCard: record.guid,
+        tempBasicCards: Object.assign({}, tempBasicCards, {
+        // Mix-in any previous values - equivalent to the store's preserveOldProperties: true,
+          [record.guid]: Object.assign({}, tempRecord, record),
+        }),
+      });
+    }
   }
 }
 

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

@@ -124,7 +124,6 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     let {
       request: {paymentOptions: {requestShipping: requestShipping}},
       savedAddresses,
-      savedBasicCards,
       selectedPayerAddress,
       selectedPaymentCard,
       selectedShippingAddress,
@@ -160,9 +159,11 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
 
     // Ensure `selectedPaymentCard` never refers to a deleted payment card and refers
     // to a payment card if one exists.
-    if (!savedBasicCards[selectedPaymentCard]) {
+    let basicCards = paymentRequest.getBasicCards(state);
+    if (!basicCards[selectedPaymentCard]) {
+      // Determining the initial selection is tracked in bug 1455789
       this.requestStore.setState({
-        selectedPaymentCard: Object.keys(savedBasicCards)[0] || null,
+        selectedPaymentCard: Object.keys(basicCards)[0] || null,
         selectedPaymentCardSecurityCode: null,
       });
     }

toolkit/components/payments/res/containers/payment-method-picker.js

@@ -5,6 +5,7 @@
 import BasicCardOption from "../components/basic-card-option.js";
 import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
 import RichSelect from "../components/rich-select.js";
+import paymentRequest from "../paymentRequest.js";
 
 /**
  * <payment-method-picker></payment-method-picker>
@@ -43,9 +44,9 @@ export default class PaymentMethodPicker extends PaymentStateSubscriberMixin(HTM
   }
 
   render(state) {
-    let {savedBasicCards} = state;
+    let basicCards = paymentRequest.getBasicCards(state);
     let desiredOptions = [];
-    for (let [guid, basicCard] of Object.entries(savedBasicCards)) {
+    for (let [guid, basicCard] of Object.entries(basicCards)) {
       let optionEl = this.dropdown.getOptionByValue(guid);
       if (!optionEl) {
         optionEl = new BasicCardOption();

toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js

@@ -47,6 +47,7 @@ export let requestStore = new PaymentsStore({
   selectedShippingOption: null,
   savedAddresses: {},
   savedBasicCards: {},
+  tempBasicCards: {},
 });
 
 

toolkit/components/payments/res/paymentRequest.js

@@ -113,10 +113,12 @@ var paymentRequest = {
     await this.domReadyPromise;
 
     log.debug("onShowPaymentRequest: domReadyPromise resolved");
+    log.debug("onShowPaymentRequest, isPrivate?", detail.isPrivate);
     document.querySelector("payment-dialog").setStateFromParent({
       request: detail.request,
       savedAddresses: detail.savedAddresses,
       savedBasicCards: detail.savedBasicCards,
+      isPrivate: detail.isPrivate,
     });
   },
 
@@ -203,6 +205,11 @@ var paymentRequest = {
     // remove listeners that may be used multiple times here
     window.removeEventListener("paymentChromeToContent", this);
   },
+
+  getBasicCards(state) {
+    let cards = Object.assign({}, state.savedBasicCards, state.tempBasicCards);
+    return cards;
+  },
 };
 
 paymentRequest.init();

toolkit/components/payments/res/paymentRequest.xhtml

@@ -27,6 +27,7 @@
   <!ENTITY basicCardPage.error.genericSave    "There was an error saving the payment card.">
   <!ENTITY basicCardPage.backButton.label     "Back">
   <!ENTITY basicCardPage.saveButton.label     "Save">
+  <!ENTITY basicCardPage.persistCheckbox.label     "Save credit card to Firefox (Security code will not be saved)">
 ]>
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
@@ -111,6 +112,7 @@
                        data-error-generic-save="&basicCardPage.error.genericSave;"
                        data-back-button-label="&basicCardPage.backButton.label;"
                        data-save-button-label="&basicCardPage.saveButton.label;"
+                       data-persist-checkbox-label="&basicCardPage.persistCheckbox.label;"
                        hidden="hidden"></basic-card-form>
     </div>
 

toolkit/components/payments/test/browser/browser_card_edit.js

@@ -20,6 +20,11 @@ add_task(async function test_add_link() {
     },
                                                           "Check add page state");
 
+    ok(!state.isPrivate,
+       "isPrivate flag is not set when paymentrequest is shown from a non-private session");
+    let persistInput = content.document.querySelector("basic-card-form labelled-checkbox");
+    ok(Cu.waiveXrays(persistInput).checked, "persist checkbox should be checked by default");
+
     let year = (new Date()).getFullYear();
     let card = {
       "cc-number": "4111111111111111",
@@ -115,3 +120,102 @@ add_task(async function test_edit_link() {
   }, args);
 });
 
+add_task(async function test_private_persist_defaults() {
+  const args = {
+    methodData: [PTU.MethodData.basicCard],
+    details: PTU.Details.total60USD,
+  };
+  await spawnInDialogForMerchantTask(PTU.ContentTasks.createRequest, async function check() {
+    let {
+      PaymentTestUtils: PTU,
+    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+    let addLink = content.document.querySelector("payment-method-picker a");
+    is(addLink.textContent, "Add", "Add link text");
+
+    addLink.click();
+
+    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+      return state.page.id == "basic-card-page" && !state.page.guid;
+    },
+                                                          "Check add page state");
+
+    ok(!state.isPrivate,
+       "isPrivate flag is not set when paymentrequest is shown from a non-private session");
+    let persistInput = content.document.querySelector("basic-card-form labelled-checkbox");
+    ok(Cu.waiveXrays(persistInput).checked,
+       "checkbox is checked by default from a non-private session");
+  }, args);
+
+  await spawnInDialogForPrivateMerchantTask(PTU.ContentTasks.createRequest, async function check() {
+    let {
+      PaymentTestUtils: PTU,
+    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+    let addLink = content.document.querySelector("payment-method-picker a");
+    is(addLink.textContent, "Add", "Add link text");
+
+    addLink.click();
+
+    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+      return state.page.id == "basic-card-page" && !state.page.guid;
+    },
+                                                          "Check add page state");
+
+    ok(state.isPrivate,
+       "isPrivate flag is set when paymentrequest is shown from a private session");
+    let persistInput = content.document.querySelector("labelled-checkbox");
+    ok(!Cu.waiveXrays(persistInput).checked,
+       "checkbox is not checked by default from a private session");
+  }, args);
+});
+
+add_task(async function test_private_card_adding() {
+  const args = {
+    methodData: [PTU.MethodData.basicCard],
+    details: PTU.Details.total60USD,
+  };
+  await spawnInDialogForPrivateMerchantTask(PTU.ContentTasks.createRequest, async function check() {
+    let {
+      PaymentTestUtils: PTU,
+    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+    let addLink = content.document.querySelector("payment-method-picker a");
+    is(addLink.textContent, "Add", "Add link text");
+
+    addLink.click();
+
+    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+      return state.page.id == "basic-card-page" && !state.page.guid;
+    },
+                                                          "Check add page state");
+
+    let savedCardCount = Object.keys(state.savedBasicCards).length;
+    let tempCardCount = Object.keys(state.tempBasicCards).length;
+
+    let year = (new Date()).getFullYear();
+    let card = {
+      "cc-number": "4111111111111111",
+      "cc-name": "J. Smith",
+      "cc-exp-month": 11,
+      "cc-exp-year": year,
+    };
+
+    info("filling fields");
+    for (let [key, val] of Object.entries(card)) {
+      let field = content.document.getElementById(key);
+      field.value = val;
+      ok(!field.disabled, `Field #${key} shouldn't be disabled`);
+    }
+
+    content.document.querySelector("basic-card-form button:last-of-type").click();
+
+    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+      return Object.keys(state.tempBasicCards).length > tempCardCount;
+    },
+                                                      "Check card was added to temp collection");
+
+    is(savedCardCount, Object.keys(state.savedBasicCards).length, "No card was saved in state");
+    is(Object.keys(state.tempBasicCards).length, 1, "Card was added temporarily");
+  }, args);
+});

toolkit/components/payments/test/browser/head.js

@@ -239,6 +239,29 @@ async function spawnInDialogForMerchantTask(merchantTaskFn, dialogTaskFn, taskAr
   });
 }
 
+async function spawnInDialogForPrivateMerchantTask(merchantTaskFn, dialogTaskFn, taskArgs, {
+  origin = "https://example.com",
+} = {
+  origin: "https://example.com",
+}) {
+  let privateWin = await BrowserTestUtils.openNewBrowserWindow({private: true});
+
+  await withMerchantTab({
+    url: origin + BLANK_PAGE_PATH,
+    browser: privateWin.gBrowser,
+  }, async merchBrowser => {
+    await ContentTask.spawn(merchBrowser, taskArgs, merchantTaskFn);
+
+    const requests = getPaymentRequests();
+    is(requests.length, 1, "Should have one payment request");
+    let request = requests[0];
+    ok(!!request.requestId, "Got a payment request with an ID");
+
+    await spawnTaskInNewDialog(request.requestId, dialogTaskFn, taskArgs);
+  });
+  await BrowserTestUtils.closeWindow(privateWin);
+}
+
 async function setupFormAutofillStorage() {
   await formAutofillStorage.initialize();
 }

toolkit/components/payments/test/mochitest/mochitest.ini

@@ -12,6 +12,7 @@ skip-if = !e10s
 [test_address_picker.html]
 [test_basic_card_form.html]
 [test_currency_amount.html]
+[test_labelled_checkbox.html]
 [test_order_details.html]
 [test_payer_address_picker.html]
 [test_payment_dialog.html]

toolkit/components/payments/test/mochitest/test_labelled_checkbox.html

@@ -0,0 +1,74 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test the labelled-checkbox component
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test the labelled-checkbox component</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script src="payments_common.js"></script>
+  <script src="../../res/vendor/custom-elements.min.js"></script>
+
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <p id="display">
+    <labelled-checkbox id="box0"></labelled-checkbox>
+    <labelled-checkbox id="box1" label="the label" value="the value"></labelled-checkbox>
+  </p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+<script type="module">
+/** Test the labelled-checkbox component **/
+
+/* import-globals-from payments_common.js */
+import "../../res/components/labelled-checkbox.js";
+
+let box0 = document.getElementById("box0");
+let box1 = document.getElementById("box1");
+
+add_task(async function test_no_values() {
+  ok(box0, "box0 exists");
+  is(box0.label, null, "Initially un-labelled");
+  is(box0.value, null, "Check .value");
+  ok(!box0.checked, "Initially is not checked");
+  ok(!box0.querySelector("input:checked"), "has no checked inner input");
+
+  box0.checked = true;
+  box0.value = "New value";
+  box0.label = "New label";
+
+  await asyncElementRendered();
+
+  ok(box0.checked, "Becomes checked");
+  ok(box0.querySelector("input:checked"), "has a checked inner input");
+  is(box0.getAttribute("label"), "New label", "Assigned label");
+  is(box0.getAttribute("value"), "New value", "Assigned value");
+});
+
+add_task(async function test_initial_values() {
+  is(box1.label, "the label", "Initial label");
+  is(box1.value, "the value", "Initial value");
+  ok(!box1.checked, "Initially unchecked");
+  ok(!box1.querySelector("input:checked"), "has no checked inner input");
+
+  box1.checked = false;
+  box1.value = "New value";
+  box1.label = "New label";
+
+  await asyncElementRendered();
+
+  ok(!box1.checked, "Checked property remains falsey");
+  is(box1.getAttribute("value"), "New value", "Assigned value");
+  is(box1.getAttribute("label"), "New label", "Assigned label");
+});
+
+</script>
+
+</body>
+</html>

toolkit/components/payments/test/mochitest/test_payment_method_picker.html

@@ -11,6 +11,7 @@ Test the payment-method-picker component
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/basic-card-option.css"/>