New issue
Advanced search Search tips

Issue 902291 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Make shippingOptions optional

Project Member Reported by jinho.b...@samsung.com, Nov 6

Issue description

In the current implementation, when calling updateWith() without
shippingOptions, reset shippingOptions as empty by force. This does not
match the behavior with the spec[1].

[1] https://w3c.github.io/payment-request/#update-a-paymentrequest-s-details-algorithm
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 8

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

commit be15a4226361caf9355ec79dfe207ad6e19b672b
Author: Jinho Bang <jinho.bang@samsung.com>
Date: Thu Nov 08 07:41:21 2018

PaymentRequest: Add a use counter for updateWith() w/o shippingOptions

This patch adds a use counter to number of calling updateWith() without
shippingOptions dictionary member on ShippingAddressChange and
ShippingOptionChange events.

Bug: 902291
Change-Id: Iac779040dc1a7245dcfe4db795e09d4ecbca9b90
Reviewed-on: https://chromium-review.googlesource.com/c/1319653
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606376}
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/chrome/browser/page_load_metrics/observers/use_counter/ukm_features.cc
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request.cc
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request.h
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request_test.cc
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request_update_event.cc
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request_update_event.h
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_request_update_event_test.cc
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/third_party/blink/renderer/modules/payments/payment_updater.h
[modify] https://crrev.com/be15a4226361caf9355ec79dfe207ad6e19b672b/tools/metrics/histograms/enums.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

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

commit 6f591c75fb11ae4f0768e8ef86b59e073487537b
Author: Jinho Bang <jinho.bang@samsung.com>
Date: Wed Dec 05 09:26:43 2018

PaymentRequest: Make shippingOptions optional in non-shipping events

Before this patch, when calling updateWith() w/o shippingOptions in
PaymentRequestUpdateEvent, it will be set to empty array by force. This
is a implementation bug and doesn't match behavior with the spec[1].
So, in case of payerdetailchange and paymentmethodchange events, it
might cause to reset the selected shipping address on payment sheet UI.
Please see the following movie:
  - https://youtu.be/7eP5DUu_m0k

So, this patch makes the shippingOptions member optional in the
payerdetailchange and paymentmethodchange events. FYI, they are still
behind runtime flags. For backward compatibility, shippingaddresschange
event and shippingoptionchange event work the same as before. It means
that this patch breaks no existing behavior.

After this patch, updateWith() works well at least in payerdetailchange
and paymentmethodchange events. Please see the following movie:
  - https://youtu.be/cBTGyJDWPGw

[1] https://w3c.github.io/payment-request/#update-a-paymentrequest-s-details-algorithm

Bug: 902291
Change-Id: I1fbdd11da49ba85a4f98bd04b97c005a95862d8a
Reviewed-on: https://chromium-review.googlesource.com/c/1280563
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613914}
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/components/payments/content/payment_request_converter.cc
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/components/payments/content/payment_request_spec.cc
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/components/payments/content/payment_request_spec_unittest.cc
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/components/payments/content/payment_request_state_unittest.cc
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/components/test/data/payments/dynamic_shipping.js
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/third_party/blink/public/mojom/payments/payment_request.mojom
[modify] https://crrev.com/6f591c75fb11ae4f0768e8ef86b59e073487537b/third_party/blink/renderer/modules/payments/payment_request.cc

Sign in to add a comment