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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by MAD
Modified:
2 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
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 25 (15 generated)
MAD
This CL fixes a few misunderstanding I had with the autofill profile APIs. Thanks! BYE ...
2 months, 1 week ago (2017-05-16 18:32:57 UTC) #4
anthonyvd
+mathp because this is pretty autofill-specific and he's the expert on that :)
2 months, 1 week 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: ...
2 months, 1 week 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; ...
2 months, 1 week 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 ...
2 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 ...
2 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
2 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)
2 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
2 months, 1 week ago (2017-05-18 01:21:20 UTC) #22
commit-bot: I haz the power
2 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973