Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2883333003: Payment request shipping address editor now properly handles countries (Closed)

Created:
11 months, 2 weeks ago by MAD
Modified:
11 months, 1 week ago
Reviewers:
Mathieu, anthonyvd
CC:
chromium-reviews, tfarina
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Payment request shipping address editor now properly handles countries The autofill profile needs the country to be set before other fields so that they can be properly internaionalized. And the country must be set using its internationalized name (as opposed to country code) when using the SetInfo method. BUG=722039, 722452 Review-Url: https://codereview.chromium.org/2883333003 Cr-Commit-Position: refs/heads/master@{#472708} Committed: https://chromium.googlesource.com/chromium/src/+/09f15955329247e16c8547119e326ac8f35a269c

Patch Set 1 #

Patch Set 2 : improved a test #

Patch Set 3 : Rebase #

Total comments: 6

Patch Set 4 : Fixed a clang compile issue #

Patch Set 5 : CR Comments 1, changing sematic of country index. #

Patch Set 6 : Rebase #

Total comments: 6

Patch Set 7 : Final nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -49 lines) Patch
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller.h View 1 2 3 4 3 chunks +15 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc View 1 2 3 4 5 6 9 chunks +113 lines, -36 lines 0 comments Download
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc View 1 2 3 4 8 chunks +28 lines, -8 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 25 (15 generated)
MAD
This CL fixes a few misunderstanding I had with the autofill profile APIs. Thanks! BYE ...
11 months, 2 weeks ago (2017-05-16 18:32:57 UTC) #4
anthonyvd
+mathp because this is pretty autofill-specific and he's the expert on that :)
11 months, 2 weeks ago (2017-05-16 19:45:07 UTC) #7
Mathieu
Autofill Expert! I reject this claim :) lgtm, thanks https://codereview.chromium.org/2883333003/diff/60001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc (right): https://codereview.chromium.org/2883333003/diff/60001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc#newcode80 chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:80: ...
11 months, 2 weeks ago (2017-05-16 20:10:06 UTC) #8
MAD
How about this now? Thanks! BYE MAD... https://codereview.chromium.org/2883333003/diff/60001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc (right): https://codereview.chromium.org/2883333003/diff/60001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc#newcode80 chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:80: autofill::CountryComboboxModel model; ...
11 months, 2 weeks ago (2017-05-17 12:38:11 UTC) #9
Mathieu
still lgtm https://codereview.chromium.org/2883333003/diff/120001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc (right): https://codereview.chromium.org/2883333003/diff/120001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc#newcode64 chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:64: const size_t kInvalidCountryIndex = static_cast<size_t>(-1); isn't size_t ...
11 months, 1 week ago (2017-05-17 19:18:31 UTC) #14
MAD
Thanks! All set. CQ'ing... BYE MAD https://codereview.chromium.org/2883333003/diff/120001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc (right): https://codereview.chromium.org/2883333003/diff/120001/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc#newcode64 chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:64: const size_t kInvalidCountryIndex ...
11 months, 1 week ago (2017-05-17 19:54:30 UTC) #15
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/2883333003/140001
11 months, 1 week ago (2017-05-17 19:56:16 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/447747)
11 months, 1 week ago (2017-05-17 22:47:53 UTC) #20
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/2883333003/140001
11 months, 1 week ago (2017-05-18 01:21:20 UTC) #22
commit-bot: I haz the power
11 months, 1 week ago (2017-05-18 06:34:18 UTC) #25
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/09f15955329247e16c8547119e32...

Powered by Google App Engine
This is Rietveld 408576698