1441752 - Add regionCode attribute to PaymentAddress

Status: RESOLVED FIXED

(Core :: DOM: Web Payments, enhancement, P1)

Description

[Marcos Caceres [:marcosc]]

PaymentAddress gained a `regionCode` attribute.

Comment 1

[Marcos Caceres [:marcosc]]

Assigning to Eden, but we are still working out the details in the spec (https://github.com/w3c/payment-request/pull/690). We also need to work out how we would actually support this, and if we should buy the data from ISO (or use Google's libaddressinput library). 

For the MVP, we are only supporting US and Canada, so it might be ok to just start with the regions of those two countries and go from there.

Comment 2

[Marcos Caceres [:marcosc]]

Gonna add the DOM side, which will just take the string (e.g., "CA" for California). I'm not sure we will do any validation in the DOM code yet.

Comment 3

[Marion Daly [:mdaly]]

P2: Assigned and actively worked on.

Comment 4

[Marco Mucci [:MarcoM]]

For assigned bugs being actively worked on we put the priority at P1.

Comment 5

[Marcos Caceres [:marcosc]]

Sorry, I got derailed by a different bug 😢 I’ll came back to this soon and set the priority appropriately when I do.

Comment 6

[Eden Chuang[:edenchuang]]

Attachment: Add regionCode attribute on PaymentAdress.

Comment 7

[Eden Chuang[:edenchuang]]

Attachment: mochitest for regionCode attribute

Comment 8

[Marcos Caceres [:marcosc]]

Thanks for picking this up, Eden.

Comment 9

[Eden Chuang[:edenchuang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eef4e3a5000c50a1d82525ad9ea01e6309fa367e

tryserver run for ensuring there is no regression fail.

Comment 10

[Eden Chuang[:edenchuang]]

Attachment: Add regionCode attribute on PaymentAdress. r=baku

rebase the patch.

Comment 11

[Eden Chuang[:edenchuang]]

Attachment: mochitest for regionCode attribute. r=baku

rebase the patch

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5a84cd8b79e
Adding an new attribute regionCode in PaymentAddress. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/37f53e939f61
mochitest for PaymentAddress.regionCode. r=baku

Comment 13

[Eliza Balazs [:ebalazs_]]

Backed out 2 changesets (Bug 1441752) for mochitest failures in dom/payments/test/test_basiccarderrors.html CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed,busted,exception,usercancel,runnable&revision=37f53e939f61efa9b11d4b4f8f3f1d36b065d79a&selectedJob=203397396

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=203397396&repo=mozilla-inbound&lineNumber=8885

Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed,busted,exception,usercancel,runnable&selectedJob=203397396&revision=3841f4ced817a06344835a744cefdf11667add7c

Comment 14

[Eden Chuang[:edenchuang]]

Attachment: mochitest for regionCode attribute. r=baku

Fix the fail on BasiccardError tests.

Comment 15

[Marcos Caceres [:marcosc]]

Comment on attachment 9014425 [details] [diff] [review]
mochitest for regionCode attribute. r=baku

Review of attachment 9014425 [details] [diff] [review]:
-----------------------------------------------------------------

I wonder if region shoube be “California”, indtead of CA?

Comment 16

[Eden Chuang[:edenchuang]]

tryserver result.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d55f1a912b5c5ddcef3e589daad494b0759d0d1b&selectedJob=203415057

(In reply to Marcos Caceres [:marcosc] from comment #15)
> Comment on attachment 9014425 [details] [diff] [review]
> mochitest for regionCode attribute. r=baku
> 
> Review of attachment 9014425 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I wonder if region shoube be “California”, indtead of CA?

I guess so, but here we do not restrict it to be.

Comment 17

[Marcos Caceres [:marcosc]]

(In reply to Eden Chuang[:edenchuang] from comment #16)
> I guess so, but here we do not restrict it to be.

Correct - it's more so just to distinguish them. We can always go back later and make those changes.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c29045d23143
Adding an new attribute regionCode in PaymentAddress. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/db490391893f
mochitest for PaymentAddress.regionCode. r=baku

Comment 19

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c29045d23143
https://hg.mozilla.org/mozilla-central/rev/db490391893f

Comment 20

[Hani Yacoub, Desktop QA]

regionCode is displayed empty in the received response whether I type CA or California in "State" field.

I'm not sure if the expected result is different than what I am verifying or what the expected behavior should be here.

Comment 21

[Eden Chuang[:edenchuang]]

This bug only provides the regionCode attribute in PaymentResponse and PaymentAddress.

I didn't implement the auto-generation of "regionCode" from "region".
That's why you get the empty "regionCode" in PaymentResponse.

We need a follow bug or reopen this bug for UI code implementation.

Comment 22

[Hani Yacoub, Desktop QA]

So should I log a bug for that?

Comment 23

[Eric Shepherd [:sheppy]]

Already documented. I added a mention to Firefox 64 for developers, though, and will have a PR submitted shortly that updates BCD for this.

Comment 24

[Matthew N. [:MattN]]

No Web Payment stuff should be in Firefox NN for developers where NN isn't Nightly as the feature isn't riding the trains so you can't even pref it on for Fx64 if you want to.

Comment 25

[Eric Shepherd [:sheppy]]

Yep, already fixed. I zoned on that one. :)

Comment 26

[Hani Yacoub, Desktop QA]

I'll change "qe-verify+" flag to "qe-verify-" since I can't verify this bug manually.