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

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: MattN, Assigned: MattN)

Tracking

Trunk
Firefox 60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [webpayments])

Attachments

(1 attachment)

By default we select a shipping address and option but should't dispatch a change event if requestShipping is false.
Comment hidden (mozreview-request)
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+
Comment hidden (mozreview-request)

Comment 5

Last year
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 hidden (mozreview-request)
Assignee

Comment 8

Last year
mozreview-review
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.

Comment 9

Last year
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)

Comment 10

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/80d2fbdf93c0
Status: ASSIGNED → RESOLVED
Closed: Last year
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.