Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false. r=jaws draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Mon, 26 Feb 2018 22:03:26 -0800
changeset 760248 81acb71c73ecd4a82dc2a0b2d5511e31f52a9f1d
parent 759603 7208b6a7b11c3ed8c87a7f17c9c30a8f9583e791
push id100586
push usermozilla@noorenberghe.ca
push dateTue, 27 Feb 2018 06:10:59 +0000
reviewersjaws
bugs1440843
milestone60.0a1
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false. r=jaws MozReview-Commit-ID: 8HfO8UN0stE
toolkit/components/payments/res/containers/payment-dialog.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
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
@@ -161,22 +161,26 @@ class PaymentDialog extends PaymentState
     }
 
     this._payButton.textContent = this._payButton.dataset[state.completionState + "Label"];
   }
 
   stateChangeCallback(state) {
     super.stateChangeCallback(state);
 
-    if (state.selectedShippingAddress != this._cachedState.selectedShippingAddress) {
-      this.changeShippingAddress(state.selectedShippingAddress);
-    }
+    // Don't dispatch change events for initial selectedShipping* changes at initialization
+    // if requestShipping is false.
+    if (state.request.paymentOptions.requestShipping) {
+      if (state.selectedShippingAddress != this._cachedState.selectedShippingAddress) {
+        this.changeShippingAddress(state.selectedShippingAddress);
+      }
 
-    if (state.selectedShippingOption != this._cachedState.selectedShippingOption) {
-      this.changeShippingOption(state.selectedShippingOption);
+      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;
--- a/toolkit/components/payments/test/PaymentTestUtils.jsm
+++ b/toolkit/components/payments/test/PaymentTestUtils.jsm
@@ -17,16 +17,22 @@ var PaymentTestUtils = {
       response.complete();
       return {
         response: response.toJSON(),
         // XXX: Bug NNN: workaround for `details` not being included in `toJSON`.
         methodDetails: response.details,
       };
     },
 
+    ensureNoPaymentRequestEvent: ({eventName}) => {
+      content.rq.addEventListener(eventName, (event) => {
+        ok(false, `Unexpected ${eventName}`);
+      });
+    },
+
     promisePaymentRequestEvent: ({eventName}) => {
       content[eventName + "Promise"] =
         new Promise(resolve => {
           content.rq.addEventListener(eventName, () => {
             resolve();
           });
         });
     },
--- a/toolkit/components/payments/test/browser/browser_change_shipping.js
+++ b/toolkit/components/payments/test/browser/browser_change_shipping.js
@@ -1,60 +1,29 @@
 "use strict";
 
-add_task(async function test_show_completePayment() {
+add_task(async function setup_profiles() {
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "add");
-  let address = {
-    "given-name": "Timothy",
-    "additional-name": "John",
-    "family-name": "Berners-Lee",
-    organization: "World Wide Web Consortium",
-    "street-address": "32 Vassar Street\nMIT Room 32-G524",
-    "address-level2": "Cambridge",
-    "address-level1": "MA",
-    "postal-code": "02139",
-    country: "US",
-    tel: "+16172535702",
-    email: "timbl@example.org",
-  };
-  profileStorage.addresses.add(address);
+  profileStorage.addresses.add(PTU.Addresses.TimBL);
   await onChanged;
 
   onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                       (subject, data) => data == "add");
-  let address2 = {
-    "given-name": "Timothy",
-    "additional-name": "John",
-    "family-name": "Berners-Lee",
-    organization: "World Wide Web Consortium",
-    "street-address": "1 Pommes Frittes Place",
-    "address-level2": "Berlin",
-    "address-level1": "BE",
-    "postal-code": "02138",
-    country: "DE",
-    tel: "+16172535702",
-    email: "timbl@example.org",
-  };
-  profileStorage.addresses.add(address2);
+  profileStorage.addresses.add(PTU.Addresses.TimBL2);
   await onChanged;
 
   onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                       (subject, data) => data == "add");
 
-  let card = {
-    "cc-exp-month": 1,
-    "cc-exp-year": 9999,
-    "cc-name": "John Doe",
-    "cc-number": "999999999999",
-  };
+  profileStorage.creditCards.add(PTU.BasicCards.JohnDoe);
+  await onChanged;
+});
 
-  profileStorage.creditCards.add(card);
-  await onChanged;
-
+add_task(async function test_change_shipping() {
   await BrowserTestUtils.withNewTab({
     gBrowser,
     url: BLANK_PAGE_URL,
   }, async browser => {
     let {win, frame} =
       await setupPaymentDialog(browser, {
         methodData: [PTU.MethodData.basicCard],
         details: PTU.Details.twoShippingOptions,
@@ -73,17 +42,17 @@ 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'");
 
-    ContentTask.spawn(browser, {
+    await 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");
@@ -101,31 +70,73 @@ add_task(async function test_show_comple
     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");
 
-    let addressLines = address2["street-address"].split("\n");
+    let addressLines = PTU.Addresses.TimBL2["street-address"].split("\n");
     let actualShippingAddress = result.response.shippingAddress;
+    let expectedAddress = PTU.Addresses.TimBL2;
     is(actualShippingAddress.addressLine[0], addressLines[0], "Address line 1 should match");
     is(actualShippingAddress.addressLine[1], addressLines[1], "Address line 2 should match");
-    is(actualShippingAddress.country, address2.country, "Country should match");
-    is(actualShippingAddress.region, address2["address-level1"], "Region should match");
-    is(actualShippingAddress.city, address2["address-level2"], "City should match");
-    is(actualShippingAddress.postalCode, address2["postal-code"], "Zip code should match");
-    is(actualShippingAddress.organization, address2.organization, "Org should match");
+    is(actualShippingAddress.country, expectedAddress.country, "Country should match");
+    is(actualShippingAddress.region, expectedAddress["address-level1"], "Region should match");
+    is(actualShippingAddress.city, expectedAddress["address-level2"], "City should match");
+    is(actualShippingAddress.postalCode, expectedAddress["postal-code"], "Zip code should match");
+    is(actualShippingAddress.organization, expectedAddress.organization, "Org should match");
     is(actualShippingAddress.recipient,
-       `${address2["given-name"]} ${address2["additional-name"]} ${address2["family-name"]}`,
-       "Recipient country should match");
-    is(actualShippingAddress.phone, address2.tel, "Phone should match");
+       `${expectedAddress["given-name"]} ${expectedAddress["additional-name"]} ` +
+       `${expectedAddress["family-name"]}`,
+       "Recipient should match");
+    is(actualShippingAddress.phone, expectedAddress.tel, "Phone should match");
 
     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");
 
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 });
+
+add_task(async function test_no_shippingchange_without_shipping() {
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let {win, frame} =
+      await setupPaymentDialog(browser, {
+        methodData: [PTU.MethodData.basicCard],
+        details: PTU.Details.twoShippingOptions,
+        merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
+      }
+    );
+
+    ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+    }, PTU.ContentTasks.ensureNoPaymentRequestEvent);
+    info("added shipping change handler");
+
+    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");
+
+    let actualShippingAddress = result.response.shippingAddress;
+    ok(actualShippingAddress === null,
+       "Check that shipping address is null with requestShipping:false");
+
+    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");
+
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+});
--- a/toolkit/components/payments/test/browser/browser_show_dialog.js
+++ b/toolkit/components/payments/test/browser/browser_show_dialog.js
@@ -74,16 +74,17 @@ add_task(async function test_show_comple
   await BrowserTestUtils.withNewTab({
     gBrowser,
     url: BLANK_PAGE_URL,
   }, async browser => {
     let {win, frame} =
       await setupPaymentDialog(browser, {
         methodData,
         details,
+        options: PTU.Options.requestShippingOption,
         merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
       }
     );
 
     info("entering CSC");
     await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.setSecurityCode, {
       securityCode: "999",
     });
@@ -111,32 +112,32 @@ 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.");
+    is(result.response.shippingOption, "2", "Check shipping option");
 
     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 {win, frame} =
       await setupPaymentDialog(browser, {
         methodData,
         details,
+        options: PTU.Options.requestShippingOption,
         merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
       }
     );
 
     await ContentTask.spawn(browser, {
       eventName: "shippingoptionchange",
     }, PTU.ContentTasks.promisePaymentRequestEvent);
 
--- a/toolkit/components/payments/test/browser/head.js
+++ b/toolkit/components/payments/test/browser/head.js
@@ -1,15 +1,15 @@
 "use strict";
 
 /* eslint
   "no-unused-vars": ["error", {
     vars: "local",
     args: "none",
-    varsIgnorePattern: "^(Cc|Ci|Cr|Cu|EXPORTED_SYMBOLS)$",
+    varsIgnorePattern: "^(EXPORTED_SYMBOLS)$",
   }],
 */
 
 
 const BLANK_PAGE_PATH = "/browser/toolkit/components/payments/test/browser/blank_page.html";
 const BLANK_PAGE_URL = "https://example.com" + BLANK_PAGE_PATH;
 
 const paymentSrv = Cc["@mozilla.org/dom/payments/payment-request-service;1"]