Bug 1427947 - Dispatch `shippingoptionchange` when the shipping option is changed. r?mattn draft
authorJared Wein <jwein@mozilla.com>
Thu, 15 Feb 2018 15:03:40 -0500
changeset 757622 4ff4e5b51cea2030cd22519cbf4492513879c170
parent 757621 b7cfa800ddaa10421ae4e44d62e7c8733dba6126
push id99802
push userbmo:jaws@mozilla.com
push dateTue, 20 Feb 2018 22:25:22 +0000
reviewersmattn
bugs1427947
milestone60.0a1
Bug 1427947 - Dispatch `shippingoptionchange` when the shipping option is changed. r?mattn MozReview-Commit-ID: J8U7ln90XnZ
toolkit/components/payments/content/paymentDialogWrapper.js
toolkit/components/payments/res/containers/payment-dialog.js
toolkit/components/payments/res/containers/shipping-option-picker.js
toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
toolkit/components/payments/res/paymentRequest.js
toolkit/components/payments/test/PaymentTestUtils.jsm
toolkit/components/payments/test/browser/browser_change_shipping.js
toolkit/components/payments/test/browser/browser_show_dialog.js
--- a/toolkit/components/payments/content/paymentDialogWrapper.js
+++ b/toolkit/components/payments/content/paymentDialogWrapper.js
@@ -399,16 +399,24 @@ var paymentDialogWrapper = {
     paymentSrv.respondPayment(showResponse);
   },
 
   async onChangeShippingAddress({shippingAddressGUID}) {
     let address = await this._convertProfileAddressToPaymentAddress(shippingAddressGUID);
     paymentSrv.changeShippingAddress(this.request.requestId, address);
   },
 
+  onChangeShippingOption({optionID}) {
+    // Note, failing here on browser_host_name.js because the test closes
+    // the dialog before the onChangeShippingOption is called, thus
+    // deleting the request and making the requestId invalid. Unclear
+    // why we aren't seeing the same issue with onChangeShippingAddress.
+    paymentSrv.changeShippingOption(this.request.requestId, optionID);
+  },
+
   /**
    * @implements {nsIObserver}
    * @param {nsISupports} subject
    * @param {string} topic
    * @param {string} data
    */
   observe(subject, topic, data) {
     switch (topic) {
@@ -433,16 +441,20 @@ var paymentDialogWrapper = {
       case "initializeRequest": {
         this.initializeFrame();
         break;
       }
       case "changeShippingAddress": {
         this.onChangeShippingAddress(data);
         break;
       }
+      case "changeShippingOption": {
+        this.onChangeShippingOption(data);
+        break;
+      }
       case "paymentCancel": {
         this.onPaymentCancel();
         break;
       }
       case "pay": {
         this.onPay(data);
         break;
       }
--- a/toolkit/components/payments/res/containers/payment-dialog.js
+++ b/toolkit/components/payments/res/containers/payment-dialog.js
@@ -75,16 +75,22 @@ class PaymentDialog extends PaymentState
   }
 
   changeShippingAddress(shippingAddressGUID) {
     paymentRequest.changeShippingAddress({
       shippingAddressGUID,
     });
   }
 
+  changeShippingOption(optionID) {
+    paymentRequest.changeShippingOption({
+      optionID,
+    });
+  }
+
   /**
    * Set some state from the privileged parent process.
    * Other elements that need to set state should use their own `this.requestStore.setState`
    * method provided by the `PaymentStateSubscriberMixin`.
    *
    * @param {object} state - See `PaymentsStore.setState`
    */
   setStateFromParent(state) {
@@ -115,41 +121,48 @@ class PaymentDialog extends PaymentState
       this.requestStore.setState({
         selectedPaymentCard: Object.keys(savedBasicCards)[0] || null,
         selectedPaymentCardSecurityCode: null,
       });
     }
 
     // Ensure `selectedShippingOption` never refers to a deleted shipping option and
     // refers to a shipping option if one exists.
-    if (!shippingOptions.find(option => option.id == selectedShippingOption)) {
+    if (shippingOptions &&
+        !shippingOptions.find(option => option.id == selectedShippingOption)) {
       // The shippingOption spec says to use the last specified selected: true item.
       for (let i = shippingOptions.length - 1; i >= 0; i--) {
         if (shippingOptions[i].selected) {
           selectedShippingOption = shippingOptions[i].id;
           break;
         }
       }
       if (!selectedShippingOption && shippingOptions.length) {
         selectedShippingOption = shippingOptions[0].id;
       }
+      this._cachedState.selectedShippingOption = selectedShippingOption;
       this.requestStore.setState({
         selectedShippingOption,
       });
     }
   }
 
   stateChangeCallback(state) {
     super.stateChangeCallback(state);
 
     if (state.selectedShippingAddress != this._cachedState.selectedShippingAddress) {
       this.changeShippingAddress(state.selectedShippingAddress);
     }
 
+    if (state.selectedShippingOption != this._cachedState.selectedShippingOption) {
+      this.changeShippingOption(state.selectedShippingOption);
+    }
+
     this._cachedState.selectedShippingAddress = state.selectedShippingAddress;
+    this._cachedState.selectedShippingOption = state.selectedShippingOption;
   }
 
   render(state) {
     let request = state.request;
     this._hostNameEl.textContent = request.topLevelPrincipal.URI.displayHost;
 
     let totalItem = request.paymentDetails.totalItem;
     let totalAmountEl = this.querySelector("#total > currency-amount");
--- a/toolkit/components/payments/res/containers/shipping-option-picker.js
+++ b/toolkit/components/payments/res/containers/shipping-option-picker.js
@@ -45,21 +45,22 @@ class ShippingOptionPicker extends Payme
     for (let option of desiredOptions) {
       this.dropdown.popupBox.appendChild(option);
     }
 
     // Update selectedness after the options are updated
     let selectedShippingOption = state.selectedShippingOption;
     let selectedOptionEl =
       this.dropdown.getOptionByValue(selectedShippingOption);
+    this.dropdown.selectedOption = selectedOptionEl;
+
     if (selectedShippingOption && !selectedOptionEl) {
       throw new Error(`Selected shipping option ${selectedShippingOption} ` +
                       `does not exist in option elements`);
     }
-    this.dropdown.selectedOption = selectedOptionEl;
   }
 
   handleEvent(event) {
     switch (event.type) {
       case "change": {
         this.onChange(event);
         break;
       }
--- a/toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
+++ b/toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
@@ -31,20 +31,20 @@ let requestStore = new PaymentsStore({
     paymentOptions: {
       requestPayerName: false,
       requestPayerEmail: false,
       requestPayerPhone: false,
       requestShipping: false,
       shippingType: "shipping",
     },
   },
-  selectedShippingOption: null,
   selectedPaymentCard: null,
   selectedPaymentCardSecurityCode: null,
   selectedShippingAddress: null,
+  selectedShippingOption: null,
   savedAddresses: {},
   savedBasicCards: {},
 });
 
 
 /* exported PaymentStateSubscriberMixin */
 
 /**
--- a/toolkit/components/payments/res/paymentRequest.js
+++ b/toolkit/components/payments/res/paymentRequest.js
@@ -108,15 +108,19 @@ var paymentRequest = {
   pay(data) {
     this.sendMessageToChrome("pay", data);
   },
 
   changeShippingAddress(data) {
     this.sendMessageToChrome("changeShippingAddress", data);
   },
 
+  changeShippingOption(data) {
+    this.sendMessageToChrome("changeShippingOption", data);
+  },
+
   onPaymentRequestUnload() {
     // remove listeners that may be used multiple times here
     window.removeEventListener("paymentChromeToContent", this);
   },
 };
 
 paymentRequest.init();
--- a/toolkit/components/payments/test/PaymentTestUtils.jsm
+++ b/toolkit/components/payments/test/PaymentTestUtils.jsm
@@ -17,20 +17,28 @@ this.PaymentTestUtils = {
       response.complete();
       return {
         response: response.toJSON(),
         // XXX: Bug NNN: workaround for `details` not being included in `toJSON`.
         methodDetails: response.details,
       };
     },
 
-    addShippingChangeHandler: ({details}) => {
-      content.rq.onshippingaddresschange = ev => {
-        ev.updateWith(details);
-      };
+    promisePaymentRequestEvent: ({eventName}) => {
+      content[eventName + "Promise"] =
+        new Promise(resolve => {
+          content.rq.addEventListener(eventName, () => {
+            resolve();
+          });
+        });
+    },
+
+    awaitPaymentRequestEventPromise: async ({eventName}) => {
+      await content[eventName + "Promise"];
+      return true;
     },
 
     /**
      * Create a new payment request and cache it as `rq`.
      *
      * @param {Object} args
      * @param {PaymentMethodData[]} methodData
      * @param {PaymentDetailsInit} details
@@ -135,48 +143,22 @@ this.PaymentTestUtils = {
     },
   },
 
   /**
    * Common PaymentDetailsInit for testing
    */
   Details: {
     total60USD: {
-      shippingOptions: [
-        {
-          id: "standard",
-          label: "Ground Shipping (2 days)",
-          amount: { currency: "USD", value: "5.00" },
-          selected: true,
-        },
-        {
-          id: "drone",
-          label: "Drone Express (2 hours)",
-          amount: { currency: "USD", value: "25.00" },
-        },
-      ],
       total: {
         label: "Total due",
         amount: { currency: "USD", value: "60.00" },
       },
     },
     total60EUR: {
-      shippingOptions: [
-        {
-          id: "standard",
-          label: "Ground Shipping (4 days)",
-          amount: { currency: "EUR", value: "7.00" },
-          selected: true,
-        },
-        {
-          id: "drone",
-          label: "Drone Express (8 hours)",
-          amount: { currency: "EUR", value: "29.00" },
-        },
-      ],
       total: {
         label: "Total due",
         amount: { currency: "EUR", value: "75.00" },
       },
     },
     twoDisplayItems: {
       total: {
         label: "Total due",
--- a/toolkit/components/payments/test/browser/browser_change_shipping.js
+++ b/toolkit/components/payments/test/browser/browser_change_shipping.js
@@ -84,27 +84,31 @@ add_task(async function test_show_comple
                                  PTU.DialogContentTasks.selectShippingOptionById,
                                  "1");
 
     shippingOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingOptions);
     is(shippingOptions.optionCount, 2, "there should be two shipping options");
     is(shippingOptions.selectedOptionValue, "1", "selected should be '1'");
 
-    await ContentTask.spawn(browser,
-                            {
-                              details: PTU.Details.twoShippingOptionsEUR,
-                            },
-                            PTU.ContentTasks.addShippingChangeHandler);
+    ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+    }, PTU.ContentTasks.promisePaymentRequestEvent);
     info("added shipping change handler");
+
     await spawnPaymentDialogTask(frame,
                                  PTU.DialogContentTasks.selectShippingAddressByCountry,
                                  "DE");
     info("changed shipping address to DE country");
 
+    await ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+    }, PTU.ContentTasks.awaitPaymentRequestEventPromise);
+    info("got shippingaddresschange event");
+
     shippingOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingOptions);
     todo_is(shippingOptions.selectedOptionCurrency, "EUR",
             "Shipping options should be in EUR when shippingaddresschange is implemented");
 
     info("clicking pay");
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.completePayment);
 
--- a/toolkit/components/payments/test/browser/browser_show_dialog.js
+++ b/toolkit/components/payments/test/browser/browser_show_dialog.js
@@ -1,12 +1,12 @@
 "use strict";
 
 const methodData = [PTU.MethodData.basicCard];
-const details = PTU.Details.total60USD;
+const details = PTU.Details.twoShippingOptions;
 
 add_task(async function test_show_abort_dialog() {
   await BrowserTestUtils.withNewTab({
     gBrowser,
     url: BLANK_PAGE_URL,
   }, async browser => {
     // start by creating a PaymentRequest, and show it
     await ContentTask.spawn(browser, {methodData, details}, PTU.ContentTasks.createAndShowRequest);
@@ -123,11 +123,57 @@ add_task(async function test_show_comple
     is(result.response.methodName, "basic-card", "Check methodName");
     let methodDetails = result.methodDetails;
     is(methodDetails.cardholderName, "John Doe", "Check cardholderName");
     is(methodDetails.cardNumber, "999999999999", "Check cardNumber");
     is(methodDetails.expiryMonth, "01", "Check expiryMonth");
     is(methodDetails.expiryYear, "9999", "Check expiryYear");
     is(methodDetails.cardSecurityCode, "999", "Check cardSecurityCode");
 
+    todo(result.response.shippingOption, "2",
+         "DOM is not sending the shipping option when it hasn't been explicitly changed.");
+
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 });
+
+add_task(async function test_show_completePayment() {
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let dialogReadyPromise = waitForWidgetReady();
+    // start by creating a PaymentRequest, and show it
+    await ContentTask.spawn(browser, {methodData, details}, PTU.ContentTasks.createAndShowRequest);
+
+    // get a reference to the UI dialog and the requestId
+    let [win] = await Promise.all([getPaymentWidget(), dialogReadyPromise]);
+    ok(win, "Got payment widget");
+    let requestId = paymentUISrv.requestIdForWindow(win);
+    ok(requestId, "requestId should be defined");
+    is(win.closed, false, "dialog should not be closed");
+
+    let frame = await getPaymentFrame(win);
+
+    ContentTask.spawn(browser, {
+      eventName: "shippingoptionchange",
+    }, PTU.ContentTasks.promisePaymentRequestEvent);
+
+    info("changing shipping option to '1' from default selected option of '2'");
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.selectShippingOptionById, "1");
+
+    await ContentTask.spawn(browser, {
+      eventName: "shippingoptionchange",
+    }, PTU.ContentTasks.awaitPaymentRequestEventPromise);
+    info("got shippingoptionchange event");
+
+    info("clicking pay");
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.completePayment);
+
+    // Add a handler to complete the payment above.
+    info("acknowledging the completion from the merchant page");
+    let result = await ContentTask.spawn(browser, {}, PTU.ContentTasks.addCompletionHandler);
+
+    is(result.response.shippingOption, "1", "Check shipping option");
+
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+});