Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2838403002:
Enable supporting arbitrary short string payment method names. (Closed)
|
Created:
3 years, 8 months ago by please use gerrit instead Modified:
3 years, 8 months ago Reviewers:
gogerald1 CC:
chromium-reviews, mahmadi+paymentswatch_chromium.org, rouslan+payments_chromium.org, agrieve+watch_chromium.org, sebsg+paymentswatch_chromium.org, gogerald+paymentswatch_chromium.org, adrian_ripple.com, zkoch+fyi_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnable supporting arbitrary short string payment method names.
Before this patch, support for "basic-card" in 3rd party Android payment
apps was hard-coded. Adding support for a new short string payment
method name would require large changes in AndroidPaymentAppFinder.java.
This patch removes custom handling for "basic-card" and instead
introduces a list of supported short string payment method names. So
far, the list contains only "basic-card".
After this patch, adding support for a new short string payment method
name is done trivially by appending it to the list of supported names.
Note that, for short string payment method names, only the names that
are published by W3C should be supported.
3rd party Android payment apps are behind a flag:
chrome://flags/#android-payment-apps
Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
Review-Url: https://codereview.chromium.org/2838403002
Cr-Commit-Position: refs/heads/master@{#467515}
Committed: https://chromium.googlesource.com/chromium/src/+/77b88c692df6e8617eabe390dee6c2f7a93e4367
Patch Set 1 #Patch Set 2 : Limit to basic-card for now #
Total comments: 6
Patch Set 3 : Update comments #Messages
Total messages: 34 (27 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from
==========
Support arbitrary short string payment method names.
Android payment apps can support arbitrary short string payment method
names by adding the following to their AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=FILEME
==========
to
==========
Support arbitrary short string payment method names.
Android payment apps can support arbitrary short string payment method
names by adding the following to their AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
rouslan@chromium.org changed reviewers: + gogerald@chromium.org
Description was changed from
==========
Support arbitrary short string payment method names.
Android payment apps can support arbitrary short string payment method
names by adding the following to their AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
to
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise with
`NotSupportedError`.
After this patch, Android payment apps can support arbitrary short string
payment method
names by adding the following to their AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
Description was changed from
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise with
`NotSupportedError`.
After this patch, Android payment apps can support arbitrary short string
payment method
names by adding the following to their AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
to
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise
with `NotSupportedError`.
After this patch, Android payment apps can support arbitrary short
string payment method names by adding the following to their
AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
Description was changed from
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise
with `NotSupportedError`.
After this patch, Android payment apps can support arbitrary short
string payment method names by adding the following to their
AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
to
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise
with `NotSupportedError`.
This patch collects the list of short payment method names from Android
payment app's AndroidManifest.xml file instead of hard-coding only
"basic-card" payment method name.
After this patch, Android payment apps can support arbitrary short
string payment method names by adding the following to their
AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
Ganggui, ptal.
lgtm lgtm,
The CQ bit was checked by rouslan@chromium.org
The CQ bit was unchecked by rouslan@chromium.org
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...
Description was changed from
==========
Support arbitrary short string payment method names.
Before this patch, calling `new PaymentRequest([{supportedMethods:
['interledger']}], details).show();` would always reject the promise
with `NotSupportedError`.
This patch collects the list of short payment method names from Android
payment app's AndroidManifest.xml file instead of hard-coding only
"basic-card" payment method name.
After this patch, Android payment apps can support arbitrary short
string payment method names by adding the following to their
AndroidManifest.xml:
<meta-data android:name="org.chromium.default_payment_method_name"
android:value="put-some-name-here" />
The short string payment method should match the following regular
expression:
^[a-z]+[a-z0-9-]*[a-z0-9]+$
The feature is behind a flag:
chrome://flags/#android-payment-apps
See design for more information:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
to
==========
Enable supporting arbitrary short string payment method names.
Before this patch, support for "basic-card" in 3rd party Android payment
apps was hard-coded. Adding support for a new short string payment
method name would require large changes in AndroidPaymentAppFinder.java.
This patch removes custom handling for "basic-card" and instead
introduces a list of supported short string payment method names. So
far, the list contains only "basic-card".
After this patch, adding support for a new short string payment method
name is done trivially by appending it to the list of supported names.
Note that, for short string payment method names, only the names that
are published by W3C should be supported.
3rd party payment apps are behind a flag:
chrome://flags/#android-payment-apps
Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE
BUG=715577
==========
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...
Description was changed from ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 ========== to ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party Android payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 ==========
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...
Patchset #2 (id:20001) has been deleted
Ganggui, would you mind taking a second look? After talking to Zach, it appears that spec will require short string payment method names to be published by W3C first before we can support them. So I've changed the code to easily support extending the list of the supported names. Currently the list has only "basic-card", so no functionality has changed.
still lgtm with three nits https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java (right): https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:37: * is a URI that starts with "https://". The "basic-card" payment method is an exception: it's a not only basic-card? https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:80: * "https://bobpay.com", "https://android.com/pay". Basic card is excluded. not only basic card is excluded anymore? https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:113: // For short string payment method names, only names published by W3C should be supported. 'short string payment method' looks not a precise description since "https://" prefixed payment method could also short. May be call it non URI based payment method?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party Android payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 ========== to ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party Android payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 ==========
The CQ bit was checked by rouslan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gogerald@chromium.org Link to the patchset: https://codereview.chromium.org/2838403002/#ps60001 (title: "Update comments")
Thank you for keeping me honest in the comments! Sending to CQ. https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java (right): https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:37: * is a URI that starts with "https://". The "basic-card" payment method is an exception: it's a On 2017/04/26 20:38:10, gogerald1 wrote: > not only basic-card? Done. https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:80: * "https://bobpay.com", "https://android.com/pay". Basic card is excluded. On 2017/04/26 20:38:10, gogerald1 wrote: > not only basic card is excluded anymore? Done. https://codereview.chromium.org/2838403002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinder.java:113: // For short string payment method names, only names published by W3C should be supported. On 2017/04/26 20:38:10, gogerald1 wrote: > 'short string payment method' looks not a precise description since "https://" > prefixed payment method could also short. > > May be call it non URI based payment method? Done.
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": 60001, "attempt_start_ts": 1493243522489950,
"parent_rev": "5674fee66841f27be43267127bd8586b89042b82", "commit_rev":
"77b88c692df6e8617eabe390dee6c2f7a93e4367"}
Message was sent while issue was closed.
Description was changed from ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party Android payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 ========== to ========== Enable supporting arbitrary short string payment method names. Before this patch, support for "basic-card" in 3rd party Android payment apps was hard-coded. Adding support for a new short string payment method name would require large changes in AndroidPaymentAppFinder.java. This patch removes custom handling for "basic-card" and instead introduces a list of supported short string payment method names. So far, the list contains only "basic-card". After this patch, adding support for a new short string payment method name is done trivially by appending it to the list of supported names. Note that, for short string payment method names, only the names that are published by W3C should be supported. 3rd party Android payment apps are behind a flag: chrome://flags/#android-payment-apps Design: https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE BUG=715577 Review-Url: https://codereview.chromium.org/2838403002 Cr-Commit-Position: refs/heads/master@{#467515} Committed: https://chromium.googlesource.com/chromium/src/+/77b88c692df6e8617eabe390dee6... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/77b88c692df6e8617eabe390dee6... |
