Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2830683002:
Log duplicate shipping option ID warning in web payments API. (Closed)
|
Created:
3 years, 8 months ago by please use gerrit instead Modified:
3 years, 8 months ago Reviewers:
Marijn Kruisselbrink CC:
blink-reviews, chromium-reviews, gogerald+paymentswatch_chromium.org, haraken, mahmadi+paymentswatch_chromium.org, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionLog duplicate shipping option ID warning in web payments API.
Before this patch, passing duplicate shipping option identifiers into
web payments API was silently treated as an indicator of invalid
shipping address. This matches the spec, but the web developer may be
confused.
The fix is to add a warning message in console:
"Duplicate shipping option identifier '<id>' is treated as an invalid
address indicator."
Spec:
https://w3c.github.io/browser-payment-api/#constructor
(See step 8, "Process shipping options.")
BUG=711677
Review-Url: https://codereview.chromium.org/2830683002
Cr-Commit-Position: refs/heads/master@{#465751}
Committed: https://chromium.googlesource.com/chromium/src/+/fed3fc6630e9ea6248153708b7f9a301b98624c4
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix test #
Messages
Total messages: 23 (16 generated)
The CQ bit was checked by rouslan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
rouslan@chromium.org changed reviewers: + mek@chromium.org
Marijn, ptal.
Description was changed from ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping identifiers are treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 ========== to ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping option identifiers are treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm, although you'll have to update the test expectations to include the new warning
https://codereview.chromium.org/2830683002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right): https://codereview.chromium.org/2830683002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:201: "Duplicate shipping option identifiers are " nit: would it make sense to include which id caused the error in the error message?
The CQ bit was checked by rouslan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Sending to cq. https://codereview.chromium.org/2830683002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right): https://codereview.chromium.org/2830683002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:201: "Duplicate shipping option identifiers are " On 2017/04/19 19:03:48, Marijn Kruisselbrink wrote: > nit: would it make sense to include which id caused the error in the error > message? Good idea. Done.
The CQ bit was unchecked by rouslan@chromium.org
The CQ bit was checked by rouslan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mek@chromium.org Link to the patchset: https://codereview.chromium.org/2830683002/#ps20001 (title: "Fix test")
The CQ bit was unchecked by rouslan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping option identifiers are treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 ========== to ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping option identifier '<id>' is treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 ==========
The CQ bit was checked by rouslan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492631227705450,
"parent_rev": "e7c580f5192add33af200317f7097b2b1b3d4910", "commit_rev":
"fed3fc6630e9ea6248153708b7f9a301b98624c4"}
Message was sent while issue was closed.
Description was changed from ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping option identifier '<id>' is treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 ========== to ========== Log duplicate shipping option ID warning in web payments API. Before this patch, passing duplicate shipping option identifiers into web payments API was silently treated as an indicator of invalid shipping address. This matches the spec, but the web developer may be confused. The fix is to add a warning message in console: "Duplicate shipping option identifier '<id>' is treated as an invalid address indicator." Spec: https://w3c.github.io/browser-payment-api/#constructor (See step 8, "Process shipping options.") BUG=711677 Review-Url: https://codereview.chromium.org/2830683002 Cr-Commit-Position: refs/heads/master@{#465751} Committed: https://chromium.googlesource.com/chromium/src/+/fed3fc6630e9ea6248153708b7f9... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/fed3fc6630e9ea6248153708b7f9... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
