Bug 1454129 - Use nsIPaymentRequest.shippingOption to determine the selected shipping option. r=sfoster draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Mon, 16 Apr 2018 22:49:28 -0700
changeset 784638 6a1ae849248a58de3e7b3774a966551bd7a63d88
parent 783746 5ded36cb383d3ccafd9b6c231c5120dcdae196a2
child 784641 c0ef39bafce4410e71a96d85ba7eec1ef3c8541f
push id107000
push usermozilla@noorenberghe.ca
push dateWed, 18 Apr 2018 22:14:39 +0000
reviewerssfoster
bugs1454129
milestone61.0a1
Bug 1454129 - Use nsIPaymentRequest.shippingOption to determine the selected shipping option. r=sfoster MozReview-Commit-ID: LaFiX34wBe8
toolkit/components/payments/res/containers/payment-dialog.js
toolkit/components/payments/res/debugging.js
toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
toolkit/components/payments/test/PaymentTestUtils.jsm
toolkit/components/payments/test/browser/browser_change_shipping.js
toolkit/components/payments/test/browser/head.js
--- a/toolkit/components/payments/res/containers/payment-dialog.js
+++ b/toolkit/components/payments/res/containers/payment-dialog.js
@@ -164,25 +164,22 @@ export default class PaymentDialog exten
       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 &&
-        !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 (shippingOptions && (!selectedShippingOption ||
+                            !shippingOptions.find(option => option.id == selectedShippingOption))) {
+      // Use the DOM's computed selected shipping option:
+      selectedShippingOption = state.request.shippingOption;
+
+      // Otherwise, default to selecting the first option:
       if (!selectedShippingOption && shippingOptions.length) {
         selectedShippingOption = shippingOptions[0].id;
       }
       this._cachedState.selectedShippingOption = selectedShippingOption;
       this.requestStore.setState({
         selectedShippingOption,
       });
     }
--- a/toolkit/components/payments/res/debugging.js
+++ b/toolkit/components/payments/res/debugging.js
@@ -67,16 +67,17 @@ let REQUEST_1 = {
   },
   paymentOptions: {
     requestPayerName: false,
     requestPayerEmail: false,
     requestPayerPhone: false,
     requestShipping: true,
     shippingType: "shipping",
   },
+  shippingOption: "456",
 };
 
 let REQUEST_2 = {
   tabId: 9,
   topLevelPrincipal: {URI: {displayHost: "example.com"}},
   requestId: "3797081f-a96b-c34b-a58b-1083c6e66e25",
   paymentMethods: [],
   paymentDetails: {
@@ -131,16 +132,17 @@ let REQUEST_2 = {
   },
   paymentOptions: {
     requestPayerName: false,
     requestPayerEmail: false,
     requestPayerPhone: false,
     requestShipping: true,
     shippingType: "shipping",
   },
+  shippingOption: "123",
 };
 
 let ADDRESSES_1 = {
   "48bnds6854t": {
     "address-level1": "MI",
     "address-level2": "Some City",
     "country": "US",
     "email": "foo@bar.com",
--- a/toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
+++ b/toolkit/components/payments/res/mixins/PaymentStateSubscriberMixin.js
@@ -33,16 +33,17 @@ export let requestStore = new PaymentsSt
     },
     paymentOptions: {
       requestPayerName: false,
       requestPayerEmail: false,
       requestPayerPhone: false,
       requestShipping: false,
       shippingType: "shipping",
     },
+    shippingOption: null,
   },
   selectedPayerAddress: null,
   selectedPaymentCard: null,
   selectedPaymentCardSecurityCode: null,
   selectedShippingAddress: null,
   selectedShippingOption: null,
   savedAddresses: {},
   savedBasicCards: {},
--- a/toolkit/components/payments/test/PaymentTestUtils.jsm
+++ b/toolkit/components/payments/test/PaymentTestUtils.jsm
@@ -112,20 +112,20 @@ var PaymentTestUtils = {
       let popupBox = Cu.waiveXrays(select).popupBox;
       let selectedOptionIndex = Array.from(popupBox.children)
                                      .findIndex(item => item.hasAttribute("selected"));
       let selectedOption = popupBox.children[selectedOptionIndex];
       let currencyAmount = selectedOption.querySelector("currency-amount");
       return {
         optionCount: popupBox.children.length,
         selectedOptionIndex,
-        selectedOptionValue: selectedOption.getAttribute("value"),
+        selectedOptionID: selectedOption.getAttribute("value"),
         selectedOptionLabel: selectedOption.getAttribute("label"),
         selectedOptionCurrency: currencyAmount.getAttribute("currency"),
-        selectedOptionAmount: currencyAmount.getAttribute("amount"),
+        selectedOptionValue: currencyAmount.getAttribute("value"),
       };
     },
 
     getShippingAddresses: () => {
       let doc = content.document;
       let addressPicker =
         doc.querySelector("address-picker[selected-state-key='selectedShippingAddress']");
       let select = addressPicker.querySelector("rich-select");
--- a/toolkit/components/payments/test/browser/browser_change_shipping.js
+++ b/toolkit/components/payments/test/browser/browser_change_shipping.js
@@ -19,46 +19,50 @@ add_task(async function test_change_ship
         merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
       }
     );
 
     let shippingOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingOptions);
     is(shippingOptions.selectedOptionCurrency, "USD", "Shipping options should be in USD");
     is(shippingOptions.optionCount, 2, "there should be two shipping options");
-    is(shippingOptions.selectedOptionValue, "2", "default selected should be '2'");
+    is(shippingOptions.selectedOptionID, "2", "default selected should be '2'");
 
     await spawnPaymentDialogTask(frame,
                                  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'");
+    is(shippingOptions.selectedOptionID, "1", "selected should be '1'");
 
     await ContentTask.spawn(browser, {
       eventName: "shippingaddresschange",
-    }, PTU.ContentTasks.promisePaymentRequestEvent);
-    info("added shipping change handler");
+      details: PTU.Details.twoShippingOptionsEUR,
+    }, PTU.ContentTasks.updateWith);
+    info("added shipping change handler to change to EUR");
 
     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");
+    is(shippingOptions.selectedOptionCurrency, "EUR",
+       "Shipping options should be in EUR after the shippingaddresschange");
+    is(shippingOptions.selectedOptionID, "1", "id:1 should still be selected");
+    is(shippingOptions.selectedOptionValue, "1.01",
+       "amount should be '1.01' after the shippingaddresschange");
 
     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.methodName, "basic-card", "Check methodName");
@@ -85,16 +89,138 @@ add_task(async function test_change_ship
     is(methodDetails.expiryMonth, "01", "Check expiryMonth");
     is(methodDetails.expiryYear, "9999", "Check expiryYear");
 
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
   await cleanupFormAutofillStorage();
 });
 
+add_task(async function test_default_shippingOptions_noneSelected() {
+  await setup();
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let shippingOptionDetails = deepClone(PTU.Details.twoShippingOptions);
+    info("make sure no shipping options are selected");
+    shippingOptionDetails.shippingOptions.forEach(opt => delete opt.selected);
+
+    let {win, frame} =
+      await setupPaymentDialog(browser, {
+        methodData: [PTU.MethodData.basicCard],
+        details: shippingOptionDetails,
+        options: PTU.Options.requestShippingOption,
+        merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
+      }
+    );
+
+    let shippingOptions =
+      await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingOptions);
+    is(shippingOptions.selectedOptionCurrency, "USD", "Shipping options should be in USD");
+    is(shippingOptions.optionCount, 2, "there should be two shipping options");
+    is(shippingOptions.selectedOptionID, "1", "default selected should be the first");
+
+    let shippingOptionDetailsEUR = deepClone(PTU.Details.twoShippingOptionsEUR);
+    info("prepare EUR options by deselecting all and giving unique IDs");
+    shippingOptionDetailsEUR.shippingOptions.forEach(opt => {
+      opt.selected = false;
+      opt.id += "-EUR";
+    });
+
+    await ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+      details: shippingOptionDetailsEUR,
+    }, PTU.ContentTasks.updateWith);
+    info("added shipping change handler to change to EUR");
+
+    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);
+    is(shippingOptions.selectedOptionCurrency, "EUR", "Shipping options should be in EUR");
+    is(shippingOptions.optionCount, 2, "there should be two shipping options");
+    is(shippingOptions.selectedOptionID, "1-EUR",
+       "default selected should be the first");
+
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+  await cleanupFormAutofillStorage();
+});
+
+add_task(async function test_default_shippingOptions_allSelected() {
+  await setup();
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let shippingOptionDetails = deepClone(PTU.Details.twoShippingOptions);
+    info("make sure no shipping options are selected");
+    shippingOptionDetails.shippingOptions.forEach(opt => opt.selected = true);
+
+    let {win, frame} =
+      await setupPaymentDialog(browser, {
+        methodData: [PTU.MethodData.basicCard],
+        details: shippingOptionDetails,
+        options: PTU.Options.requestShippingOption,
+        merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
+      }
+    );
+
+    let shippingOptions =
+      await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingOptions);
+    is(shippingOptions.selectedOptionCurrency, "USD", "Shipping options should be in USD");
+    is(shippingOptions.optionCount, 2, "there should be two shipping options");
+    is(shippingOptions.selectedOptionID, "2", "default selected should be the last selected=true");
+
+    let shippingOptionDetailsEUR = deepClone(PTU.Details.twoShippingOptionsEUR);
+    info("prepare EUR options by selecting all and giving unique IDs");
+    shippingOptionDetailsEUR.shippingOptions.forEach(opt => {
+      opt.selected = true;
+      opt.id += "-EUR";
+    });
+
+    await ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+      details: shippingOptionDetailsEUR,
+    }, PTU.ContentTasks.updateWith);
+    info("added shipping change handler to change to EUR");
+
+    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);
+    is(shippingOptions.selectedOptionCurrency, "EUR", "Shipping options should be in EUR");
+    is(shippingOptions.optionCount, 2, "there should be two shipping options");
+    is(shippingOptions.selectedOptionID, "2-EUR",
+       "default selected should be the last selected=true");
+
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+  await cleanupFormAutofillStorage();
+});
+
 add_task(async function test_no_shippingchange_without_shipping() {
   await setup();
   await BrowserTestUtils.withNewTab({
     gBrowser,
     url: BLANK_PAGE_URL,
   }, async browser => {
     let {win, frame} =
       await setupPaymentDialog(browser, {
--- a/toolkit/components/payments/test/browser/head.js
+++ b/toolkit/components/payments/test/browser/head.js
@@ -250,8 +250,12 @@ function cleanupFormAutofillStorage() {
 
 add_task(async function setup_head() {
   await setupFormAutofillStorage();
   registerCleanupFunction(function cleanup() {
     paymentSrv.cleanup();
     cleanupFormAutofillStorage();
   });
 });
+
+function deepClone(obj) {
+  return JSON.parse(JSON.stringify(obj));
+}