New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 640847 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Don’t mention “ISO 20022 CurrencyAnd30Amount” is PaymentRequest exception message

Reported by sideshowbarker@gmail.com, Aug 25 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2834.0 Safari/537.36

Steps to reproduce the problem:
1.  Open https://sideshowbarker.net/tests/payment-request/invalid-value.html

What is the expected behavior?
You should see this:

Test passes

The Payment Request API spec defines requirements for the value field of a PaymentCurrencyAmount without any reference to “ISO 20022 CurrencyAnd30Amount”.
Called PaymentRequest constructor with value "FOO" for the value field:

    new PaymentRequest([{supportedMethods: ['foo']}], {total: {label: 'bar', amount: {currency: 'USD', value: 'FOO'}}})
As expected, error message does not mention “ISO 20022 CurrencyAnd30Amount”.

What went wrong?
Instead of the expected behavior I get this:

Test fails

The Payment Request API spec defines requirements for the value field of a PaymentCurrencyAmount without any reference to “ISO 20022 CurrencyAnd30Amount”.
Called PaymentRequest constructor with value "FOO" for the value field:

    new PaymentRequest([{supportedMethods: ['foo']}], {total: {label: 'bar', amount: {currency: 'USD', value: 'FOO'}}})
Expected browser to throw with error message that does not mention “ISO 20022 CurrencyAnd30Amount”, but instead browser threw:
    TypeError: Failed to construct 'PaymentRequest': 'FOO' is not a valid ISO 20022 CurrencyAnd30Amount

Did this work before? N/A 

Chrome version: 54.0.2834.0  Channel: canary
OS Version: OS X 10.11.6
Flash Version:
 
Components: Blink>Payments
Can't reproduce. The test passes for me on Stable, Canary(54.0.2838.0) and also on 54.0.2834.0. 
Cc: sanjoy....@samsung.com
Labels: -OS-Mac OS-All
Owner: rouslan@chromium.org
Status: Assigned (was: Unconfirmed)
I know how to reproduce.

Code responsible:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp?rcl=0&l=29
Incidentally comparing the source at https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp to the requirements for the PaymentAddress interface in the spec at https://w3c.github.io/browser-payment-api/#paymentaddress-interface I notice some other parts of that source that don’t map to any requirements in the current spec.

* https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp?rcl=0&l=34 The spec does not state any normative requirements for the 'country" field. It can be any string, including the empty string (and incidentally, the spec does not make even an informative reference to ISO 3166 country codes; instead it just says, “CLDR (Common Locale Data Repository) region code”.

* https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp?rcl=0&l=45 The spec does not state any normative requirements for the 'languageCode" field. It can be any string, including the empty string (and incidentally, the spec does not make even an informative reference to ISO 639 language codes; instead it just says, “BCP-47 language code”.

* https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp?rcl=0&l=56 The PaymentAddress interface defined in the spec does not contain anything that would map to a ISO 15924 script code, so this seems like dead code. 
Cc: tkonch...@chromium.org
Labels: Needs-Feedback
Unable to reproduce the issue on win10, mac 10.11.6 and Linux 14.04 chrome version 54.0.2834.0 and canary 54.0.2838.3

Please find the screenshot

rouslan@, Could you please let us know if this can be reproducible from test team end.
Screen Shot 2016-08-25 at 1.53.37 PM.png
120 KB View Download
I guess PaymentRequest API is not enabled. Please check if "Experimental Web Features" is enabled in chrome://flags. 
tkonchada@, It's enabled only on Android at the moment. You should be able to reproduce it there. (Version 53+)
Owner: sanjoy....@samsung.com
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25156023212b2fa380ccb48b2597549119003ec2

commit 25156023212b2fa380ccb48b2597549119003ec2
Author: sanjoy.pal <sanjoy.pal@samsung.com>
Date: Thu Sep 01 01:12:12 2016

Accept any string for currency code in PaymentRequest.

BUG= 636723 ,  640847 

Review-Url: https://codereview.chromium.org/2271113002
Cr-Commit-Position: refs/heads/master@{#415831}

[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/chrome/android/java/src/org/chromium/chrome/browser/payments/CurrencyStringFormatter.java
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/LayoutTests/payments/payment-request-interface.html
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/Source/modules/payments/PaymentRequestDetailsTest.cpp
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/Source/modules/payments/PaymentsValidators.h
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp
[modify] https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2/third_party/WebKit/public/platform/modules/payments/payment_request.mojom

rouslan@, shall we change the implementation as suggested in comment#3 which is

* Allow any string for "country" including the empty string.
* Allow any string for "languageCode" including the empty string.
* Remove isValidScriptCodeFormat 
No, let's not make these changes. The "country" and "langaugeCode" fields come from the browser and go the website. It's up to the browser to decide what it deems valid here. Let's keep it as is.
sanjoy.pal@, you can change the error messages to refer to CLDR instead of ISO3166 and BCP-47 instead of ISO639.
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2465b1d8154ac1b482c0e3ec86df89d33fb9af6e

commit 2465b1d8154ac1b482c0e3ec86df89d33fb9af6e
Author: sanjoy.pal <sanjoy.pal@samsung.com>
Date: Thu Sep 15 16:56:50 2016

Modify error messages for payment address validation as per specification.

Specification
https://w3c.github.io/browser-payment-api/

BUG= 640847 

Review-Url: https://codereview.chromium.org/2340743002
Cr-Commit-Position: refs/heads/master@{#418888}

[modify] https://crrev.com/2465b1d8154ac1b482c0e3ec86df89d33fb9af6e/third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp

Status: Fixed (was: Started)

Sign in to add a comment