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

Issue 785429 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

PaymentRequest API throws on Chrome iOS when using a Number instead of a String as a `displayItem` `amount.value`

Reported by naoufalk...@gmail.com, Nov 15 2017

Issue description

Steps to reproduce the problem:
1. Create a PaymentRequest and use a `Number` for an `amount.value`.
2. PaymentRequest will throw.

What is the expected behavior?
The PaymentRequest should be created without throwing an exception.

What went wrong?
PaymentRequest throws with `TypeError: Total amount value must be a string`.

This gets called from `validatePaymentCurrencyAmount`

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 61.0.3163.100  Channel: stable
OS Version: 62.0.3202.70
Flash Version: 

The PaymentRequest API specifies that `total` should be a String. However, Chrome Desktop and Android allow Strings or Numbers to be used interchangeably. 

https://www.w3.org/TR/payment-request/#dom-paymentitem
 
payment-request-error-repo.html
415 bytes View Download
Screen Shot 2017-11-15 at 12.07.33 PM.png
128 KB View Download
Owner: mahmadi@chromium.org
Status: Assigned (was: Unconfirmed)
Moe: Please coerce numbers into strings to match the behavior of desktop and Android.
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2017

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

commit 98d5858a3a3f25586a02feb3b9160fd292817bb5
Author: Mohamad Ahmadi <mahmadi@chromium.org>
Date: Tue Nov 28 19:50:12 2017

[PR] Convert numbers->strings to match Desktop/Android validation behavior

Bug:  785429 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I23c008943b60e89b9b58265cdad3293b3dda2bb5
Reviewed-on: https://chromium-review.googlesource.com/793983
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519803}
[modify] https://crrev.com/98d5858a3a3f25586a02feb3b9160fd292817bb5/ios/chrome/browser/web/resources/payment_request.js
[modify] https://crrev.com/98d5858a3a3f25586a02feb3b9160fd292817bb5/third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 8 2017

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

commit 3efccf1f0eb24e6c8fba6a28bd3425b4ce98b82a
Author: Mohamad Ahmadi <mahmadi@chromium.org>
Date: Fri Dec 08 23:09:16 2017

[PR] Converts the currency amount number to string in JS and C++

crrev.com/c/793983 previously fixed the valition error without fixing the
problem of converting the currency amount from JS to C++.
This CL addresses that issue by converting the numbers to strings in JS.

Bug:  785429 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic28d218ad7f4122266cd3ac463e1d912a5b295a2
Reviewed-on: https://chromium-review.googlesource.com/818014
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522908}
[modify] https://crrev.com/3efccf1f0eb24e6c8fba6a28bd3425b4ce98b82a/ios/chrome/browser/web/resources/payment_request.js

Cc: durgapandey@chromium.org
Labels: M-64 Merge-Request-64
+Durga

Requesting merge of the last CL into M-64 to resolve an issue affecting some merchants (e.g., Kogan.com)  
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 10 2017

Labels: -Merge-Request-64 Hotlist-Merge-Approved Merge-Approved-64
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 11 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9f97ff49c7ff72bda078cb2668eef64af1b9858

commit d9f97ff49c7ff72bda078cb2668eef64af1b9858
Author: Mohamad Ahmadi <mahmadi@chromium.org>
Date: Mon Dec 11 15:41:03 2017

[PR] Converts the currency amount number to string in JS and C++

crrev.com/c/793983 previously fixed the valition error without fixing the
problem of converting the currency amount from JS to C++.
This CL addresses that issue by converting the numbers to strings in JS.

Bug:  785429 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic28d218ad7f4122266cd3ac463e1d912a5b295a2
Reviewed-on: https://chromium-review.googlesource.com/818014
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522908}(cherry picked from commit 3efccf1f0eb24e6c8fba6a28bd3425b4ce98b82a)
Reviewed-on: https://chromium-review.googlesource.com/819670
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#127}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/d9f97ff49c7ff72bda078cb2668eef64af1b9858/ios/chrome/browser/web/resources/payment_request.js

Issue 793311 has been merged into this issue.

Sign in to add a comment