1495549 - Retry with shipping validation on https://rsolomakhin.github.io/pr/shipping-validation/ doesn't work properly

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, defect, P1)

Description

[Matthew N. [:MattN]]

STR:
1) Visit https://rsolomakhin.github.io/pr/shipping-validation/
2) Click Buy
3) Choose a valid shipping address without a ZIP code of 12345
4) Enter a CVV for an existing card
5) Click Pay
6) The processing state appears temporarily
7) The shipping zip error appears under the shipping address picker and the UI is unlocked.
8) Click Edit for the shipping address picker
9) Change the ZIP code to 12345
10) Click Update

Expected result:
* Returned to the payment summary page
* The merchant error should get removed (maybe after some seconds)
* The Pay button should become re-enabled

Actual result:
* Returned to the payment summary page
* The merchant error remains under the shipping address picker
* The Pay button remains disabled

I'm not sure if this is a Firefox bug or a test-case bug:

The front-end causes a shippingaddresschange event to be dispatched (I didn't confirm the DOM actually dispatches it) but the test page isn't listening for it anyways so doesn't call updateWith to remove the error. Should the front-end clear the error on its own in this case or is the test page just wrong for not using updateWith to clear the error?

Comment 1

[Marcos Caceres [:marcosc]]

Attachment: Reduced test case

Comment 2

[Marcos Caceres [:marcosc]]

> I'm not sure if this is a Firefox bug or a test-case bug:

It's a bug with us. The Pay button is not being re-enabled unless `ev.updateWith()` get called:

```
request.addEventListener("shippingaddresschange", ev => {
 ev.updateWith(details);
});
```

That should not be required. 

The test itself doesn't have the code above - which is why the payment sheet locks up. 

Adding Eden to cc list.

Comment 3

[Marcos Caceres [:marcosc]]

I've attached a test case proving that it's us and showing where the bug is.

Comment 4

[Marcos Caceres [:marcosc]]

> Should the front-end clear the error on its own in this case or is the test page just wrong for not using updateWith to clear the error?

We should clear the error after the user hits "Update" in the UI. If there is a problem, then the page should call `.updateWith({shippingAddressErrors: {postalCode: "actually, it should be 54321"}})`.

If `updateWith()` is not called, the retryPromise should resolve, allowing for revalidation by the merchant:


```
async function validateResponse(response) {
  const {
    shippingAddress: { postalCode },
  } = response;

  if (postalCode === "12345") return; // valid!

  await response.retry({
    shippingAddress: { postalCode: "The postal code should be 12345." },
  });
  await validateResponse(response);
}
```

Comment 5

[Matthew N. [:MattN]]

(In reply to Marcos Caceres [:marcosc] (triage duty) from comment #4)
> > Should the front-end clear the error on its own in this case or is the test page just wrong for not using updateWith to clear the error?
> 
> We should clear the error after the user hits "Update" in the UI.

The user can't click Update in our UI until they address all errors :P We'll need to address that probably

Taking this for now to investigate and talk to UX.

Comment 6

[Matthew N. [:MattN]]

Attachment: Bug 1495549 - Clear merchant shipping errors when the shipping address changes. r=jaws

Comment 7

[Matthew N. [:MattN]]

Attachment: Bug 1495549 - Clear merchant payer errors when the payer address changes. r=jaws

Depends on D12605

Comment 8

[Matthew N. [:MattN]]

Attachment: Bug 1495549 - Clear paymentMethod errors when the paymentMethod changes. r=jaws

Depends on D12606

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d06fc29f4203
Clear merchant shipping errors when the shipping address changes. r=jaws
https://hg.mozilla.org/integration/autoland/rev/4fdd2c0651cf
Clear merchant payer errors when the payer address changes. r=jaws
https://hg.mozilla.org/integration/autoland/rev/61e7d1a58f5e
Clear paymentMethod errors when the paymentMethod changes. r=jaws

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d06fc29f4203
https://hg.mozilla.org/mozilla-central/rev/4fdd2c0651cf
https://hg.mozilla.org/mozilla-central/rev/61e7d1a58f5e

Comment 11

[Timea Cernea [:tbabos][inactive]]

Verified Fixed on latest Nightly 66.0a1 (2018-12-12) (64-bit) on Windows 10, Ubuntu 16.04 and Mac OS 10.13.