Closed Bug 1440843 Opened 6 years ago Closed 6 years ago

Don't dispatch shipping*change events if requestShipping is false

Categories

(Firefox :: WebPayments UI, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

(Whiteboard: [webpayments])

Attachments

(1 file)

By default we select a shipping address and option but should't dispatch a change event if requestShipping is false.
Comment on attachment 8953692 [details]
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

https://reviewboard.mozilla.org/r/222924/#review228854

::: toolkit/components/payments/test/browser/browser_change_shipping.js:3
(Diff revision 2)
>  "use strict";
>  
> -add_task(async function test_show_completePayment() {
> +const address = {

This can be replaced with PTU.Addresses.TimBL

::: toolkit/components/payments/test/browser/browser_change_shipping.js:17
(Diff revision 2)
> -    email: "timbl@example.org",
> +  email: "timbl@example.org",
> -  };
> +};
> -  profileStorage.addresses.add(address);
> -  await onChanged;
>  
> -  onChanged = TestUtils.topicObserved("formautofill-storage-changed",
> +const address2 = {

PTU.Addresses.TimBL2

::: toolkit/components/payments/test/browser/browser_change_shipping.js:31
(Diff revision 2)
> -    email: "timbl@example.org",
> +  email: "timbl@example.org",
> -  };
> +};
> -  profileStorage.addresses.add(address2);
> -  await onChanged;
>  
> -  onChanged = TestUtils.topicObserved("formautofill-storage-changed",
> +const card = {

PTU.BasicCards.JohnDoe
Attachment #8953692 - Flags: review?(jaws) → review+
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/0ec115a54885
Don't dispatch shipping*change events if requestShipping is false. r=jaws
Backed out changeset 0ec115a54885 (bug 1440843) for failing browser chrome on toolkit/components/payments/test/browser/browser_change_shipping.js

Link to the failures log https://treeherder.mozilla.org/logviewer.html#?job_id=164141939&repo=autoland&lineNumber=12031

Backout revision https://hg.mozilla.org/integration/autoland/rev/71ce94bb5d5ce7d464ebd676197c1003977e7d76

Failing push https://hg.mozilla.org/integration/autoland/rev/0ec115a548858e4ce55d271dc900387a934476d9
Flags: needinfo?(MattN+bmo)
Comment on attachment 8953692 [details]
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

https://reviewboard.mozilla.org/r/222924/#review229460

::: toolkit/components/payments/test/PaymentTestUtils.jsm:25
(Diff revision 4)
> +    ensureNoPaymentRequestEvent: ({eventName}) => {
> +      content.rq.addEventListener(eventName, (event) => {
> +        ok(false, `Unexpected ${eventName}`);
> +      });
> +    },

IIUC, the leak was from ContentTask.jsm holding on to the deferred until the promise was resolved. I didn't see a great generic way to clean that up so instead I made a new task helper that uses `ok(false, …)` directly.
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/80d2fbdf93c0
Don't dispatch shipping*change events if requestShipping is false. r=jaws
Flags: needinfo?(MattN+bmo)
https://hg.mozilla.org/mozilla-central/rev/80d2fbdf93c0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Whiteboard: [webpayments]
Product: Toolkit → Firefox
Target Milestone: mozilla60 → Firefox 60
You need to log in before you can comment on or make changes to this bug.