1494917 - PaymentResponse.retry({}) and PaymentResponse.retry() should not return AbortError.

Status: NEW

(Core :: DOM: Web Payments, enhancement, P3)

Description

[Eden Chuang[:edenchuang]]

In current implmentation. PaymentResponse.retry({}) and PaymentResponse.retry() return a rejected promise with AbortError. This is because no errors retry might be a not good UX for user.

However, UX team has a perfect solution for it. I am going to remove these code, and let PaymentResponse() and PaymentResponse({}) can be handled by UI component.

Comment 1

[Eden Chuang[:edenchuang]]

Attachment: Remove the MerchantValidationErrors checking when calling PaymentResponse::Retry()

Comment 2

[Marcos Caceres [:marcosc]]

Please don't merge this yet. It's come up for discussion at the W3C and we've not concluded what to do yet.

Comment 3

[Marcos Caceres [:marcosc]]

Related spec issues:
https://github.com/w3c/payment-request/issues/792

And: 
https://github.com/w3c/payment-request/issues/793

Comment 4

[Eden Chuang[:edenchuang]]

Marcos, Is any updates on the spec discussion?

Comment 5

[Marcos Caceres [:marcosc]]

Yes, but I've not had time to update the spec - sorry :( I'll make this top priority once I'm back from TPAC (in a week).

Comment 6

[Marcos Caceres [:marcosc]]

So, the conclusion was on the spec side was to console.warn on the bogus cases.

Comment 7

[Eden Chuang[:edenchuang]]

Attachment: Warning on brower console when calling PaymentResponse.retry with empty errros or non-empty payer errors if requestPayerXXX is false.

Update the patch according to the spec modification.

For calling PaymentResponse.retry with empty error fields and with non-empty payer errors if corresponding PaymentOptions.requestPayerXXX is false, warning on browser console to inform the developer.

Comment 8

[Eden Chuang[:edenchuang]]

Attachment: mochitest updation.

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 9029487 [details] [diff] [review]
Warning on brower console when calling PaymentResponse.retry with empty errros or non-empty payer errors if requestPayerXXX is false.

Review of attachment 9029487 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +353,4 @@
>  IDBOpenDBOptions_StorageTypeWarning=The ‘storage’ attribute in options passed to indexedDB.open is deprecated and will soon be removed. To get persistent storage, please use navigator.storage.persist() instead.
>  DOMQuadBoundsAttrWarning=DOMQuad.bounds is deprecated in favor of DOMQuad.getBounds()
>  UnsupportedEntryTypesIgnored=Ignoring unsupported entryTypes: %S.
> +# LOCALIZATION NOTE: Do not translate "PaymentRequest". The %S is the id of the PaymentRequest.

LOCALIZATION NODE(PaymentRequestRetryWithEmptyErrors): ...

@@ +355,5 @@
>  UnsupportedEntryTypesIgnored=Ignoring unsupported entryTypes: %S.
> +# LOCALIZATION NOTE: Do not translate "PaymentRequest". The %S is the id of the PaymentRequest.
> +PaymentRequestRetryWithEmptyErrors=retrying the PaymentRequest(%S) with empty error fields.
> +# LOCALIZATION NOTE: Do not translate "PaymentRequest", "PayerErrorFields.name" and "PaymentOptions.requestPayerName". The %S is the id of the PaymentRequest
> +PaymentRequestRetryWithNonemptyPayerErrorName=retrying the PaymentRequest(%S) with non-empty PayerErrorFields.name, but PaymentOptions.requestPayerName is false. 

extra space.

Comment 10

[Eden Chuang[:edenchuang]]

Attachment: Warning on brower console when calling PaymentResponse.retry with empty errros or non-empty payer errors if requestPayerXXX is false.

Update patch according to reviewer's comment.
Update patch with new C++ coding style.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14cfc544bea0
mochitest test updation. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cdcc397c971
Warning to console when calling PaymentResponse.retry() with improper error fields. r=baku

Comment 12

[Eliza Balazs [:ebalazs_]]

Backed out 2 changesets (Bug 1494917) for bc failures in browser/components/payments/test/browser/browser_address_edit.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&revision=1cdcc397c9717d719d628dbc76b80f982c31279d&selectedJob=218118635

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=218118635&repo=mozilla-inbound

Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&revision=dc54ea11b0ece580cf965bae3d37f5ef4f0ee623