[Webkit-unassigned] [Bug 229295] New: [PaymentRequest] Discrepancies / bugs in PaymentResponse.retry() implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=229295

            Bug ID: 229295
           Summary: [PaymentRequest] Discrepancies / bugs in
                    PaymentResponse.retry() implementation
           Product: WebKit
           Version: Safari 14
          Hardware: All
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: New Bugs
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: p-o.blouin at ssense.com

Created attachment 435877

  --> https://bugs.webkit.org/attachment.cgi?id=435877&action=review

Retry with generic error / set error on contact

Tested on iOS 14 + MacOS 11.5.2
Safari 14.1.2

Hello! I think there are discrepancies in how Webkit implemented the retry method in PaymentResponse versus the official W3C's documentation (https://w3c.github.io/payment-request/#retry-method)

There are a few problems with the PaymentValidationErrors.

1. The `error` field.

As per official documentation, the error field should behave like this:

```
error member
  A general description of an error with the payment from which the user can attempt to recover.
  For example, the user may recover by retrying the payment. A developer can optionally pass the
  error member on its own to give a general overview of validation issues, or it can be passed in
  combination with other members of the PaymentValidationErrors dictionary.
```

Following this, when I run the following code, I should expect that Apple Pay displays a generic error using that string in the payment sheet.
What happen is that the error is set on the contact field and the string in not even displayed (see attachment).

```
await paymentResponse.retry({
  error: 'Invalid Payment Method, please try again',
});
```

Another case that is not working for me, is when I try to use the `paymentMethod` member to set an error for the payment method. I tried a few way to make it work, but it always crash my PaymentRequest with a `TypeError`. Not sure why, I don't have much information in Safari's dev console.

```
await paymentResponse.retry({
    paymentMethod: new window.ApplePayError('unknown'),
});
```

I digged through the implementation in `https://github.com/WebKit/webkit/blob/main/Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp` and from what I understood, I think the problem lies in `ApplePayPaymentHandler::computeAddressErrors` which is called in `ApplePayPaymentHandler::retry`.

I think the logic is just wrong, because in the rety method, we take the `error` field from the PaymentValidationErrors and we send that to `computeAddressErrors(WTFMove(validationErrors.error), WTFMove(validationErrors.shippingAddress), errors);`.
Then in that method, when validationErrors.error is not null we set an error on the shipping contact. We should only set an error on the shipping contact if any `shippingAddress` errors are sent.

When the user send only an `error` field, I would expect that the payment sheet displays a generic error, unreleated to the contact information.


To resume, 2 problems:
  - I think the implementation of the error handling in `ApplePayPaymentHandler::retry` is wrong
  - Sending `paymentMethod` error seems to crash


I wish I could provide a way to reproduce, but since I can't share my code and also that the setup for Apple Pay is a bit complicated that wouldn't work. If there is an easy way to test that use case and provide a way to reproduce please let me know and I will do it.

Thank you

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210819/f7b10896/attachment.htm>