Change text and disable empty combobox for payment request shipping address

Change text and disable empty combobox for payment request shipping address

BUG=721500
Review-Url: https://codereview.chromium.org/2888733004
Cr-Commit-Position: refs/heads/master@{#473186}
Committed: https://chromium.googlesource.com/chromium/src/+/5022bd5a5c78faa35e4236abb37dd67b86b9cf44

[MAD, 2017-05-17 19:40:16]

mad@chromium.org changed reviewers:
+ sebsg@chromium.org

[MAD, 2017-05-17 19:40:17]

Plz...

Thanks!

BYE
MAD

[MAD, 2017-05-17 19:41:56]

mad@chromium.org changed reviewers:
+ anthonyvd@chromium.org

[MAD, 2017-05-17 19:41:57]

anthonyvd@chromium.org: Please review changes in 

chrome\browser\ui\views\payments

[MAD, 2017-05-17 20:02:43]

The CQ bit was checked by mad@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-17 20:04:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 20:53:23]

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

[commit-bot: I haz the power, 2017-05-17 20:53:24]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sebsg, 2017-05-17 20:55:02]

lgtm

[MAD, 2017-05-18 00:48:03]

My initial fix was breaking some tests, so I came up with another way that is
more specific.

Thanks!

BYE
MAD
P.S.: And I need the OWNER review of Anthony anyway. :-)

[MAD, 2017-05-18 14:26:14]

The CQ bit was checked by mad@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-18 14:26:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 15:01:37]

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

[commit-bot: I haz the power, 2017-05-18 15:01:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sebsg, 2017-05-18 15:05:45]

Still lgtm for Autofill code. :)

[anthonyvd, 2017-05-18 15:08:44]

lgtm % couple small things

https://codereview.chromium.org/2888733004/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2888733004/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:342:
static_cast<views::Combobox*>(dialog()->GetViewByID(kBillingAddressType));
I don't think you need to cast, SetEnabled should be a function on views::View

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
File components/autofill_strings.grdp (left):

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
components/autofill_strings.grdp:358: <message
name="IDS_AUTOFILL_ADD_BILLING_ADDRESS" desc="The string to display in
comboboxes when no billing addresses are available.">
Are you sure this isn't used on Android (they used a different syntax so code
search doesn't find it)

[sebsg, 2017-05-18 15:15:16]

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
File components/autofill_strings.grdp (left):

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
components/autofill_strings.grdp:358: <message
name="IDS_AUTOFILL_ADD_BILLING_ADDRESS" desc="The string to display in
comboboxes when no billing addresses are available.">
On 2017/05/18 15:08:44, anthonyvd wrote:
> Are you sure this isn't used on Android (they used a different syntax so code
> search doesn't find it)

Here is how I usually check:

Usually when it's used in java it has this in the <message>:
formatter_data="android_java".

and the syntax is the very similar, no caps and no IDS_ prefix.

So in this case I think we are good :)

[MAD, 2017-05-18 16:09:14]

Answered the comments, and changed a DCHECK to an if() because, for some reason,
some bots had a null combobox in some tests, even after filling the credit card
view.

Any idea why?

Thanks!

BYE
MAD

https://codereview.chromium.org/2888733004/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2888733004/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:342:
static_cast<views::Combobox*>(dialog()->GetViewByID(kBillingAddressType));
On 2017/05/18 15:08:43, anthonyvd wrote:
> I don't think you need to cast, SetEnabled should be a function on views::View

But I also need to call GetRowCount(). :-)

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
File components/autofill_strings.grdp (left):

https://codereview.chromium.org/2888733004/diff/40001/components/autofill_str...
components/autofill_strings.grdp:358: <message
name="IDS_AUTOFILL_ADD_BILLING_ADDRESS" desc="The string to display in
comboboxes when no billing addresses are available.">
On 2017/05/18 15:15:16, sebsg wrote:
> On 2017/05/18 15:08:44, anthonyvd wrote:
> > Are you sure this isn't used on Android (they used a different syntax so
code
> > search doesn't find it)
> 
> Here is how I usually check:
> 
> Usually when it's used in java it has this in the <message>:
> formatter_data="android_java".
> 
> and the syntax is the very similar, no caps and no IDS_ prefix.
> 
> So in this case I think we are good :)

Thanks Seb.

[MAD, 2017-05-18 16:09:23]

The CQ bit was checked by mad@chromium.org

[MAD, 2017-05-18 16:09:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org,
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/2888733004/#ps80001
(title: "Some bots failed that DCHECK.")

[commit-bot: I haz the power, 2017-05-18 16:10:21]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 16:56:05]

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

[commit-bot: I haz the power, 2017-05-18 16:56:07]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[MAD, 2017-05-18 19:11:00]

The CQ bit was checked by mad@chromium.org

[MAD, 2017-05-18 19:11:01]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org,
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/2888733004/#ps100001
(title: "Fixed a weird goof")

[commit-bot: I haz the power, 2017-05-18 19:11:48]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 22:20:03]

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

[commit-bot: I haz the power, 2017-05-18 22:20:04]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[MAD, 2017-05-19 02:08:03]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-05-19 02:08:47]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 03:50:41]

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

[commit-bot: I haz the power, 2017-05-19 03:50:42]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[MAD, 2017-05-19 11:27:08]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-05-19 11:27:26]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 13:21:05]

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

[commit-bot: I haz the power, 2017-05-19 13:21:07]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[MAD, 2017-05-19 13:47:45]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-05-19 13:47:58]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 14:45:13]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495201665440500,
"parent_rev": "d77cd716740a94334fc7c98749876f4a013b8322", "commit_rev":
"5022bd5a5c78faa35e4236abb37dd67b86b9cf44"}

[commit-bot: I haz the power, 2017-05-19 14:45:23]

Description was changed from

==========
Change text and disable empty combobox for payment request shipping address

BUG=721500
==========

to

==========
Change text and disable empty combobox for payment request shipping address

BUG=721500

Review-Url: https://codereview.chromium.org/2888733004
Cr-Commit-Position: refs/heads/master@{#473186}
Committed:
https://chromium.googlesource.com/chromium/src/+/5022bd5a5c78faa35e4236abb37d...
==========

[commit-bot: I haz the power, 2017-05-19 14:45:24]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/5022bd5a5c78faa35e4236abb37d...