autofill/options: address some nits I had on one of rouslan@'s CLs[1].

autofill/options: address some nits I had on one of rouslan@'s CLs[1].

R=rouslan@chromium.org
BUG=none

[1] https://chromiumcodereview.appspot.com/243013004/#msg38

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270260

[Dan Beam, 2014-05-07 04:10:04]

sorry, i'm getting a little stir crazy...

[please use gerrit instead, 2014-05-07 05:10:14]

code lgtm. comments: please fix.

https://codereview.chromium.org/273533003/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/autofill_edit_address_overlay.js (right):

https://codereview.chromium.org/273533003/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/autofill_edit_address_overlay.js:336: * @param
{{languageCode: string, components: Array.<Object>}} input Info
"components" is an array of array of objects. Example for US:

components:
[[{field: 'fullName', length: 'long', placeholder: 'Add name'}],
 [{field: 'addrLines', length: 'long', placeholder: 'Add line'}],
 [{field: 'city', length: 'short'},
  {field: 'state', length: 'short'},
  {field: 'postalCode', length: 'short'}]]

See for examples:

http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/optio...

https://codereview.chromium.org/273533003/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/autofill_edit_address_overlay.js:352: * @param
{!Array.<{{field: string, value: string, placeholder: string=}}>}
An array of arrays of objects with elements "field", "length", and
"placeholder". (Not "value".)

See the previous comment for an example.

[please use gerrit instead, 2014-05-07 05:12:57]

https://codereview.chromium.org/273533003/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/autofill_options_handler.cc (right):

https://codereview.chromium.org/273533003/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/autofill_options_handler.cc:165:
scoped_ptr<base::ListValue> default_country_components(new base::ListValue);
Don't code c++ and js at the same time, kids.

[Dan Beam, 2014-05-07 20:28:23]

https://chromiumcodereview.appspot.com/273533003/diff/60001/chrome/browser/re...
File chrome/browser/resources/options/autofill_edit_address_overlay.js (right):

https://chromiumcodereview.appspot.com/273533003/diff/60001/chrome/browser/re...
chrome/browser/resources/options/autofill_edit_address_overlay.js:336: * @param
{{languageCode: string, components: Array.<Object>}} input Info
On 2014/05/07 05:10:14, Rouslan Solomakhin wrote:
> "components" is an array of array of objects. Example for US:
> 
> components:
> [[{field: 'fullName', length: 'long', placeholder: 'Add name'}],
>  [{field: 'addrLines', length: 'long', placeholder: 'Add line'}],
>  [{field: 'city', length: 'short'},
>   {field: 'state', length: 'short'},
>   {field: 'postalCode', length: 'short'}]]
> 
> See for examples:
> 
>
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/optio...

i think in general this is different from how the native dialog handles this
info and slightly duplicative but too lazy to update right now.  added TODO().

https://chromiumcodereview.appspot.com/273533003/diff/60001/chrome/browser/re...
chrome/browser/resources/options/autofill_edit_address_overlay.js:352: * @param
{!Array.<{{field: string, value: string, placeholder: string=}}>}
On 2014/05/07 05:10:14, Rouslan Solomakhin wrote:
> An array of arrays of objects with elements "field", "length", and
> "placeholder". (Not "value".)
> 
> See the previous comment for an example.

Done.

[Dan Beam, 2014-05-07 20:28:27]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-05-07 20:34:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dbeam@chromium.org/273533003/70001

[commit-bot: I haz the power, 2014-05-08 06:45:15]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
  win_chromium_rel on tryserver.chromium

[commit-bot: I haz the power, 2014-05-08 19:06:43]

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

[commit-bot: I haz the power, 2014-05-08 19:06:43]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[please use gerrit instead, 2014-05-08 21:01:24]

An awful lot of webui browser tests failed on the windows bot. Coincidence?

[Dan Beam, 2014-05-09 00:25:59]

On 2014/05/08 21:01:24, Rouslan Solomakhin wrote:
> An awful lot of webui browser tests failed on the windows bot. Coincidence?

yes, see "browser_tests (without patch)" has the same failures and read the logs

[Dan Beam, 2014-05-09 00:26:05]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-05-09 00:30:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dbeam@chromium.org/273533003/70001

[commit-bot: I haz the power, 2014-05-09 06:14:36]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-09 08:56:40]

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

[commit-bot: I haz the power, 2014-05-09 08:56:40]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Dan Beam, 2014-05-13 22:18:11]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-05-13 22:18:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dbeam@chromium.org/273533003/70001

[commit-bot: I haz the power, 2014-05-14 00:11:59]

Change committed as 270260