Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

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

Created:
3 years, 7 months ago by MAD
Modified:
3 years, 7 months 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 ...
3 years, 7 months ago (2017-05-16 18:32:57 UTC) #4
anthonyvd
+mathp because this is pretty autofill-specific and he's the expert on that :)
3 years, 7 months 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: ...
3 years, 7 months 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; ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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
3 years, 7 months 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)
3 years, 7 months 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
3 years, 7 months ago (2017-05-18 01:21:20 UTC) #22
commit-bot: I haz the power
3 years, 7 months 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