Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2236933007:
Always fire 'shippingaddresschange' event. (Closed)
|
Created:
4 years, 4 months ago by please use gerrit instead Modified:
4 years, 4 months ago Reviewers:
gone CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAlways trigger 'shippingaddresschange' event.
If the user is changing their shipping address, fire the
'shippingaddresschange' event unconditionally to better comply with the
spec: "The shipping address changed algorithm runs when the user provides a
new shipping address."
https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm
BUG=632716
Committed: https://crrev.com/26266594edf7704a496236ed58fe6f2a9b667d74
Cr-Commit-Position: refs/heads/master@{#412122}
Patch Set 1 #Patch Set 2 : Fix test failure by changing how notifyReadyToPay() is fired #
Total comments: 4
Patch Set 3 : Initializations in constructors #
Total comments: 2
Patch Set 4 : @link #
Messages
Total messages: 25 (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: + dfalcantara@chromium.org
dfalcantara, ptal.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
Description was changed from ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, trigger the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 ========== to ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, fire the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 ==========
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://chromiumcodereview.appspot.com/2236933007/diff/20001/chrome/android/j... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (left): https://chromiumcodereview.appspot.com/2236933007/diff/20001/chrome/android/j... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:785: PaymentRequestUI.TYPE_SHIPPING_ADDRESSES, mShippingAddressesSection); Does the UI not need to be updated anymore? https://chromiumcodereview.appspot.com/2236933007/diff/20001/chrome/android/j... File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://chromiumcodereview.appspot.com/2236933007/diff/20001/chrome/android/j... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:221: private final android.os.Handler mHandler = new android.os.Handler(); Should be explicit about what thread this Handler is running on. It'll be tied to whatever thread you construct it on, otherwise, which isn't always clear.
The CQ bit was checked by rouslan@chromium.org to run a CQ dry run
ptal patch 2. https://codereview.chromium.org/2236933007/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (left): https://codereview.chromium.org/2236933007/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:785: PaymentRequestUI.TYPE_SHIPPING_ADDRESSES, mShippingAddressesSection); On 2016/08/15 20:56:34, dfalcantara wrote: > Does the UI not need to be updated anymore? At this point in code, the spinner is showing. The website is checking the shipping address. Once the website finishes verifying the address, it will update the total price and shipping options via PaymentRequestImpl.updateWith() method on line 426. That method will update the UI. https://codereview.chromium.org/2236933007/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2236933007/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:221: private final android.os.Handler mHandler = new android.os.Handler(); On 2016/08/15 20:56:34, dfalcantara wrote: > Should be explicit about what thread this Handler is running on. It'll be tied > to whatever thread you construct it on, otherwise, which isn't always clear. Moved initializations into constructors, which are called on UI thread.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ptal patch 3, actually.
lgtm https://codereview.chromium.org/2236933007/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2236933007/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1033: * PaymentRequestImpl.complete(int result) method. All other callers {@link PaymentRequestImpl#complete(int)}.
Sending to cq. https://codereview.chromium.org/2236933007/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2236933007/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1033: * PaymentRequestImpl.complete(int result) method. All other callers On 2016/08/15 23:51:48, dfalcantara wrote: > {@link PaymentRequestImpl#complete(int)}. Done.
The CQ bit was checked by rouslan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dfalcantara@chromium.org Link to the patchset: https://codereview.chromium.org/2236933007/#ps60001 (title: "@link")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, fire the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 ========== to ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, fire the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, fire the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 ========== to ========== Always trigger 'shippingaddresschange' event. If the user is changing their shipping address, fire the 'shippingaddresschange' event unconditionally to better comply with the spec: "The shipping address changed algorithm runs when the user provides a new shipping address." https://w3c.github.io/browser-payment-api/#shipping-address-changed-algorithm BUG=632716 Committed: https://crrev.com/26266594edf7704a496236ed58fe6f2a9b667d74 Cr-Commit-Position: refs/heads/master@{#412122} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/26266594edf7704a496236ed58fe6f2a9b667d74 Cr-Commit-Position: refs/heads/master@{#412122} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
