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

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

By default we select a shipping address and option but should't dispatch a change event if requestShipping is false.

Comment 1

[Matthew N. [:MattN]]

Attachment: Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

Review commit: https://reviewboard.mozilla.org/r/222924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222924/

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8953692 [details]
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222924/diff/1-2/

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

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: "[e-mail]",
> +  email: "[e-mail]",
> -  };
> +};
> -  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: "[e-mail]",
> +  email: "[e-mail]",
> -  };
> +};
> -  profileStorage.addresses.add(address2);
> -  await onChanged;
>  
> -  onChanged = TestUtils.topicObserved("formautofill-storage-changed",
> +const card = {

PTU.BasicCards.JohnDoe

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8953692 [details]
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222924/diff/2-3/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ec115a54885
Don't dispatch shipping*change events if requestShipping is false. r=jaws

Comment 6

[Arthur Iakab [arthur_iakab]]

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

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8953692 [details]
Bug 1440843 - Don't dispatch shipping*change events if requestShipping is false.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222924/diff/3-4/

Comment 8

[Matthew N. [:MattN]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80d2fbdf93c0
Don't dispatch shipping*change events if requestShipping is false. r=jaws

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/80d2fbdf93c0