Issue 2271113002: Accept any string for currency code in PaymentRequest.

pals

Patchset #2 (id:20001) has been deleted

4 years, 3 months ago (2016-08-24 09:35:29 UTC) #1

pals

sanjoy.pal@samsung.com changed reviewers: + rouslan@chromium.org

4 years, 3 months ago (2016-08-25 06:47:46 UTC) #2

pals

Description was changed from ========== Accept any string for currency code in PaymentRequest. BUG=636723 ========== ...

4 years, 3 months ago (2016-08-25 11:44:02 UTC) #5

please use gerrit instead

Good start! https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java (right): https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java#newcode532 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:532: private static final String ELLIPSIS = "..."; ...

4 years, 3 months ago (2016-08-25 16:47:38 UTC) #6

haraken

haraken@chromium.org changed reviewers: + haraken@chromium.org

4 years, 3 months ago (2016-08-26 01:20:04 UTC) #7

haraken

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl File third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl (right): https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl#newcode10 third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl:10: [TreatNullAs=NullString] required DOMString currency; On 2016/08/25 16:47:38, rouslan wrote: ...

4 years, 3 months ago (2016-08-26 01:20:05 UTC) #8

pals

PTAL. https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java (right): https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java#newcode532 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:532: private static final String ELLIPSIS = "..."; On ...

4 years, 3 months ago (2016-08-26 07:20:26 UTC) #9

please use gerrit instead

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java (right): https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java#newcode532 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:532: private static final String ELLIPSIS = "..."; On 2016/08/26 ...

4 years, 3 months ago (2016-08-26 17:01:37 UTC) #10

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
(right):

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:532:
private static final String ELLIPSIS = "...";
On 2016/08/26 07:20:26, pals wrote:
> On 2016/08/25 16:47:38, rouslan wrote:
> > Use the unicode ellipsis character:
> > http://www.fileformat.info/info/unicode/char/2026/index.htm
> 
> If it is ok, I'll do the application related changes as a follow up patch?

OK. Since Samsung does not use Chromium's UI (as far as I know), you don't have
to modify the UI code aside from making it compile and making sure that all
tests pass. I can take care of making it look good.

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/junit/s...
File
chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java
(left):

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/junit/s...
chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java:78:
{"55.00", "USDr", "en-US", null, ExpectedValidity.INVALID_AMOUNT_CURRENCY_CODE},
On 2016/08/26 07:20:26, pals wrote:
> On 2016/08/25 16:47:38, rouslan wrote:
> > Once you move currency truncation into CurrenycStringFormatter, please add a
> few
> > test cases that check the truncation logic.
> 
> If it is ok, I'll do the application related changes as a follow up patch?

OK

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl
(right):

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl:10:
[TreatNullAs=NullString] required DOMString currency;
OK

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp
(right):

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp:38: for
(int i = 0; i < 2049; i++)
On 2016/08/26 07:20:26, pals wrote:
> On 2016/08/25 16:47:38, rouslan wrote:
> > i < 2048
> 
> One extra space for '\0'. strlen() does not include '\0'.

In the current state, the last value if "i" is "2048". The last operation of the
for-loop is

longString[2048] = 'a';

However, on the next line you overwrite this value immediately:

longString[2048] = '\0';

It's not a matter of efficiency, but it's strange to write to the same location
twice in a row.

pals

PTAL. I'm working on the UI related changes here https://codereview.chromium.org/2281913002/ https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java (right): https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java#newcode532 ...

4 years, 3 months ago (2016-08-27 11:16:31 UTC) #11

PTAL. 

I'm working on the UI related changes here
https://codereview.chromium.org/2281913002/

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
(right):

https://codereview.chromium.org/2271113002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:532:
private static final String ELLIPSIS = "...";
On 2016/08/26 17:01:37, rouslan wrote:
> On 2016/08/26 07:20:26, pals wrote:
> > On 2016/08/25 16:47:38, rouslan wrote:
> > > Use the unicode ellipsis character:
> > > http://www.fileformat.info/info/unicode/char/2026/index.htm
> > 
> > If it is ok, I'll do the application related changes as a follow up patch?
> 
> OK. Since Samsung does not use Chromium's UI (as far as I know), you don't
have
> to modify the UI code aside from making it compile and making sure that all
> tests pass. I can take care of making it look good.

I'm working on it here https://codereview.chromium.org/2281913002/.

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp
(right):

https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp:38: for
(int i = 0; i < 2049; i++)
On 2016/08/26 17:01:37, rouslan wrote:
> On 2016/08/26 07:20:26, pals wrote:
> > On 2016/08/25 16:47:38, rouslan wrote:
> > > i < 2048
> > 
> > One extra space for '\0'. strlen() does not include '\0'.
> 
> In the current state, the last value if "i" is "2048". The last operation of
the
> for-loop is
> 
> longString[2048] = 'a';
> 
> However, on the next line you overwrite this value immediately:
> 
> longString[2048] = '\0';
> 
> It's not a matter of efficiency, but it's strange to write to the same
location
> twice in a row.

I'm sorry. You are correct. Rectified the issue.

pals

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

4 years, 3 months ago (2016-08-27 11:16:58 UTC) #12

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/180001

4 years, 3 months ago (2016-08-27 11:17:09 UTC) #13

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 3 months ago (2016-08-27 12:53:17 UTC) #14

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 3 months ago (2016-08-27 12:53:18 UTC) #15

please use gerrit instead

rouslan@chromium.org changed reviewers: + tsepez@chromium.org

4 years, 3 months ago (2016-08-27 15:30:14 UTC) #16

please use gerrit instead

1) Please update the comment in mojom interface definition: https://cs.chromium.org/chromium/src/third_party/WebKit/public/platform/modules/payments/payment_request.mojom?rcl=0&l=39 +tsepez to review mojom. 2) ...

4 years, 3 months ago (2016-08-27 15:30:14 UTC) #17

pals

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

4 years, 3 months ago (2016-08-29 04:46:43 UTC) #18

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/200001

4 years, 3 months ago (2016-08-29 04:46:58 UTC) #19

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 3 months ago (2016-08-29 06:13:49 UTC) #20

commit-bot: I haz the power

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_android_rel_ng/builds/131641)

4 years, 3 months ago (2016-08-29 06:13:50 UTC) #21

please use gerrit instead

You're very close :-) https://codereview.chromium.org/2271113002/diff/200001/chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java File chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java (right): https://codereview.chromium.org/2271113002/diff/200001/chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java#newcode106 chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java:106: private static String longString2048() { ...

4 years, 3 months ago (2016-08-29 14:23:40 UTC) #23

Tom Sepez

Ok, my only concern is whether we inject this string back into an HTML context ...

4 years, 3 months ago (2016-08-29 16:42:02 UTC) #24

please use gerrit instead

On 2016/08/29 16:42:02, Tom Sepez wrote: > Ok, my only concern is whether we inject ...

4 years, 3 months ago (2016-08-29 17:10:39 UTC) #25

Tom Sepez

> > We don't inject this string into HTML anywhere. Ok, LGTM.

4 years, 3 months ago (2016-08-29 17:13:32 UTC) #26

pals

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

4 years, 3 months ago (2016-08-30 03:28:18 UTC) #27

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/220001

4 years, 3 months ago (2016-08-30 03:45:44 UTC) #28

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 3 months ago (2016-08-30 05:16:36 UTC) #29

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 3 months ago (2016-08-30 05:16:37 UTC) #30

pals

Please take another look. https://codereview.chromium.org/2271113002/diff/200001/chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java File chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java (right): https://codereview.chromium.org/2271113002/diff/200001/chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java#newcode106 chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java:106: private static String longString2048() { ...

4 years, 3 months ago (2016-08-30 09:02:10 UTC) #31

haraken

haraken@chromium.org changed reviewers: + bashi@chromium.org, yukishiino@chromium.org

4 years, 3 months ago (2016-08-30 09:38:26 UTC) #32

haraken

Sorry about the late reply... > https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl > File third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl > (right): > > https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl#newcode10 ...

4 years, 3 months ago (2016-08-30 09:38:27 UTC) #33

Yuki

On 2016/08/30 09:38:27, haraken wrote: > Sorry about the late reply... > > > > ...

4 years, 3 months ago (2016-08-30 10:23:50 UTC) #34

pals

On 2016/08/30 10:23:50, Yuki wrote: > On 2016/08/30 09:38:27, haraken wrote: > > Sorry about ...

4 years, 3 months ago (2016-08-30 13:20:04 UTC) #35

On 2016/08/30 10:23:50, Yuki wrote:
> On 2016/08/30 09:38:27, haraken wrote:
> > Sorry about the late reply...
> > 
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl:10:
> > > [TreatNullAs=NullString] required DOMString currency;
> > > On 2016/08/26 01:20:05, haraken wrote:
> > > > On 2016/08/25 16:47:38, rouslan wrote:
> > > > > Let's not allow nulls. At least an empty string should be specified.
> > > > 
> > > > Right, [TreatNullAs] must be accompanied with EmptyString per the spec:
> > > > 
> > > > https://www.w3.org/TR/WebIDL/#TreatNullAs
> > > 
> > > ['Null currency code should throw', null, function() {
> > >             new PaymentRequest([{'supportedMethods': ['foo']}],
> > > buildDetails(detailNames[i], {'currency': null}))
> > >         }],
> > > 
> > > For this test case, the value of |item.amount().currency()| in [1] is
coming
> > as
> > > "null", so its not throwing any exception.
> > > 
> > > [1]
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...
> > > 
> > 
> > I think that's an expected behavior. null is normally converted to "null"
for
> a
> > non-nullable DOMString. Maybe you want to use 'DOMString?' instead?
> > 
> > [TreatNullAs=NullString] is deprecated, so we shouldn't use it.
> > 
> > +bashi +yukishiino would be more familiar with this.
> 
> [TreatNullAs=EmptyString] is defined in Web IDL, so you shouldn't use it
unless
> the spec specifies.  Otherwise, it's spec violation.
> 
> Also, Payment spec specifies that
>     required DOMString currency;
> so you shouldn't use 'DOMString?' (nullable DOMString), either.
> 
> Following Web IDL and Payment spec, |null| should be converted to
> ToString(null), which is "null".
> See https://tc39.github.io/ecma262/#sec-tostring

We don't want to allow { currency: null }. So, we should check for "null" and
throw an exception instead of changing the IDL file. Is that correct?

haraken

On 2016/08/30 13:20:04, pals wrote: > On 2016/08/30 10:23:50, Yuki wrote: > > On 2016/08/30 ...

4 years, 3 months ago (2016-08-30 14:18:00 UTC) #36

On 2016/08/30 13:20:04, pals wrote:
> On 2016/08/30 10:23:50, Yuki wrote:
> > On 2016/08/30 09:38:27, haraken wrote:
> > > Sorry about the late reply...
> > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > > File
third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl:10:
> > > > [TreatNullAs=NullString] required DOMString currency;
> > > > On 2016/08/26 01:20:05, haraken wrote:
> > > > > On 2016/08/25 16:47:38, rouslan wrote:
> > > > > > Let's not allow nulls. At least an empty string should be specified.
> > > > > 
> > > > > Right, [TreatNullAs] must be accompanied with EmptyString per the
spec:
> > > > > 
> > > > > https://www.w3.org/TR/WebIDL/#TreatNullAs
> > > > 
> > > > ['Null currency code should throw', null, function() {
> > > >             new PaymentRequest([{'supportedMethods': ['foo']}],
> > > > buildDetails(detailNames[i], {'currency': null}))
> > > >         }],
> > > > 
> > > > For this test case, the value of |item.amount().currency()| in [1] is
> coming
> > > as
> > > > "null", so its not throwing any exception.
> > > > 
> > > > [1]
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...
> > > > 
> > > 
> > > I think that's an expected behavior. null is normally converted to "null"
> for
> > a
> > > non-nullable DOMString. Maybe you want to use 'DOMString?' instead?
> > > 
> > > [TreatNullAs=NullString] is deprecated, so we shouldn't use it.
> > > 
> > > +bashi +yukishiino would be more familiar with this.
> > 
> > [TreatNullAs=EmptyString] is defined in Web IDL, so you shouldn't use it
> unless
> > the spec specifies.  Otherwise, it's spec violation.
> > 
> > Also, Payment spec specifies that
> >     required DOMString currency;
> > so you shouldn't use 'DOMString?' (nullable DOMString), either.
> > 
> > Following Web IDL and Payment spec, |null| should be converted to
> > ToString(null), which is "null".
> > See https://tc39.github.io/ecma262/#sec-tostring
> 
> We don't want to allow { currency: null }. So, we should check for "null" and
> throw an exception instead of changing the IDL file. Is that correct?

Why do you not want to allow { currency: null }? As yukishiino@ mentioned, per
the spec it should be allowed.

please use gerrit instead

On 2016/08/30 14:18:00, haraken wrote: > On 2016/08/30 13:20:04, pals wrote: > > On 2016/08/30 ...

4 years, 3 months ago (2016-08-30 16:30:54 UTC) #37

On 2016/08/30 14:18:00, haraken wrote:
> On 2016/08/30 13:20:04, pals wrote:
> > On 2016/08/30 10:23:50, Yuki wrote:
> > > On 2016/08/30 09:38:27, haraken wrote:
> > > > Sorry about the late reply...
> > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > > > File
> third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl
> > > > > (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2271113002/diff/140001/third_party/WebKit/Sou...
> > > > >
third_party/WebKit/Source/modules/payments/PaymentCurrencyAmount.idl:10:
> > > > > [TreatNullAs=NullString] required DOMString currency;
> > > > > On 2016/08/26 01:20:05, haraken wrote:
> > > > > > On 2016/08/25 16:47:38, rouslan wrote:
> > > > > > > Let's not allow nulls. At least an empty string should be
specified.
> > > > > > 
> > > > > > Right, [TreatNullAs] must be accompanied with EmptyString per the
> spec:
> > > > > > 
> > > > > > https://www.w3.org/TR/WebIDL/#TreatNullAs
> > > > > 
> > > > > ['Null currency code should throw', null, function() {
> > > > >             new PaymentRequest([{'supportedMethods': ['foo']}],
> > > > > buildDetails(detailNames[i], {'currency': null}))
> > > > >         }],
> > > > > 
> > > > > For this test case, the value of |item.amount().currency()| in [1] is
> > coming
> > > > as
> > > > > "null", so its not throwing any exception.
> > > > > 
> > > > > [1]
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...
> > > > > 
> > > > 
> > > > I think that's an expected behavior. null is normally converted to
"null"
> > for
> > > a
> > > > non-nullable DOMString. Maybe you want to use 'DOMString?' instead?
> > > > 
> > > > [TreatNullAs=NullString] is deprecated, so we shouldn't use it.
> > > > 
> > > > +bashi +yukishiino would be more familiar with this.
> > > 
> > > [TreatNullAs=EmptyString] is defined in Web IDL, so you shouldn't use it
> > unless
> > > the spec specifies.  Otherwise, it's spec violation.
> > > 
> > > Also, Payment spec specifies that
> > >     required DOMString currency;
> > > so you shouldn't use 'DOMString?' (nullable DOMString), either.
> > > 
> > > Following Web IDL and Payment spec, |null| should be converted to
> > > ToString(null), which is "null".
> > > See https://tc39.github.io/ecma262/#sec-tostring
> > 
> > We don't want to allow { currency: null }. So, we should check for "null"
and
> > throw an exception instead of changing the IDL file. Is that correct?
> 
> Why do you not want to allow { currency: null }? As yukishiino@ mentioned, per
> the spec it should be allowed.

Alright, let's follow the spec exactly. Please remove [TreatNullAs...].

pals

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

4 years, 3 months ago (2016-08-31 04:36:24 UTC) #39

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/240001

4 years, 3 months ago (2016-08-31 04:36:39 UTC) #40

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 3 months ago (2016-08-31 06:32:01 UTC) #41

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 3 months ago (2016-08-31 06:32:02 UTC) #42

pals

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org Link to the ...

4 years, 3 months ago (2016-08-31 14:25:51 UTC) #46

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/240001

4 years, 3 months ago (2016-08-31 14:26:11 UTC) #47

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 3 months ago (2016-08-31 14:32:16 UTC) #48

commit-bot: I haz the power

Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/250206)

4 years, 3 months ago (2016-08-31 14:32:17 UTC) #49

pals

sanjoy.pal@samsung.com changed reviewers: + dfalcantara@chromium.org

4 years, 3 months ago (2016-08-31 14:46:23 UTC) #50

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2271113002/240001

4 years, 3 months ago (2016-09-01 01:08:27 UTC) #54

commit-bot: I haz the power

Description was changed from ========== Accept any string for currency code in PaymentRequest. BUG=636723, 640847 ...

4 years, 3 months ago (2016-09-01 01:12:29 UTC) #55

commit-bot: I haz the power

Description was changed from ========== Accept any string for currency code in PaymentRequest. BUG=636723, 640847 ...

4 years, 3 months ago (2016-09-01 01:14:35 UTC) #57

commit-bot: I haz the power

4 years, 3 months ago (2016-09-01 01:14:38 UTC) #58

Message was sent while issue was closed.

Patchset 11 (id:??) landed as
https://crrev.com/25156023212b2fa380ccb48b2597549119003ec2
Cr-Commit-Position: refs/heads/master@{#415831}

Issue 2271113002: Accept any string for currency code in PaymentRequest. (Closed)

Description

Patch Set 1 #

Patch Set 2 : Test #

Patch Set 3 : Ellipsize #

Patch Set 4 : Ellipsize after 6 #

Patch Set 5 : Added tests #

Patch Set 6 : Fixed 640847 #

Patch Set 7 : minus java changes #

Patch Set 8 : Fixed issues #

Patch Set 9 : Added browser side validation #

Patch Set 10 : StringBuilder #

Patch Set 11 : Allow null as per spec #

Messages