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

VERIFIED FIXED in Firefox 66

Status

()

P1
normal
VERIFIED FIXED
3 months ago
3 days ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

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

Firefox Tracking Flags

(firefox66 verified)

Details

(Whiteboard: [webpayments], URL)

User Story

From talking to UX:
1) Clear shippingAddressErrors when the selectedShippingAddress changes (likewise for other addresses)
2) Clear shippingAddressErrors when the the selectedShippingAddress is edited (likewise for other addresses)
* Use timeLastModified?

Attachments

(4 attachments)

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?
Flags: qe-verify+
Flags: needinfo?(mcaceres)
Created attachment 9014687 [details]
Reduced test case
Flags: needinfo?(mcaceres)
> 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.
I've attached a test case proving that it's us and showing where the bug is.
> 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);
}
```
Whiteboard: [webpayments] [triage] → [webpayments]
(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.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Priority: P2 → P1
User Story: (updated)
Created attachment 9026833 [details]
Bug 1495549 - Clear merchant shipping errors when the shipping address changes. r=jaws
Created attachment 9026835 [details]
Bug 1495549 - Clear merchant payer errors when the payer address changes. r=jaws

Depends on D12605
Attachment #9026833 - Attachment description: Bug 1495549 - Clear merchant shipping errors when the shipping address changes. r=jaws,sfoster → Bug 1495549 - Clear merchant shipping errors when the shipping address changes. r=jaws
Attachment #9026835 - Attachment description: Bug 1495549 - Clear merchant payer errors when the payer address changes. → Bug 1495549 - Clear merchant payer errors when the payer address changes. r=jaws
Created attachment 9028720 [details]
Bug 1495549 - Clear paymentMethod errors when the paymentMethod changes. r=jaws

Depends on D12606
Attachment #9028720 - Attachment description: Bug 1495549 - WIP: Clear paymentMethod errors when the paymentMethod changes. → Bug 1495549 - Clear paymentMethod errors when the paymentMethod changes. r=jaws

Comment 9

4 days ago
Pushed by mozilla@noorenberghe.ca:
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

4 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d06fc29f4203
https://hg.mozilla.org/mozilla-central/rev/4fdd2c0651cf
https://hg.mozilla.org/mozilla-central/rev/61e7d1a58f5e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 days ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66

Comment 11

3 days ago
Verified Fixed on latest Nightly 66.0a1 (2018-12-12) (64-bit) on Windows 10, Ubuntu 16.04 and Mac OS 10.13.
Status: RESOLVED → VERIFIED
status-firefox66: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.