Maintain focused column in chrome://settings/searchEngines

Maintain focused column in chrome://settings/searchEngines

1. Focused column is maintained when pressing up/down in list to change
   selected list item.
2. Shift+Tab from outside of the list now goes to the last list item
   field instead of the first one.

This CL is part of splitting https://codereview.chromium.org/664583006/
into several smaller CLs. All applicable reviewer comments from that CL
have been addressed.

This CL depends on https://codereview.chromium.org/683813004/

BUG=428428
Committed: https://crrev.com/39698bd5cf1a61e67796b9a855f32321a1d29242
Cr-Commit-Position: refs/heads/master@{#303933}

[bondd, 2014-10-29 21:14:20]

bondd@chromium.org changed reviewers:
+ dbeam@chromium.org

[bondd, 2014-10-29 21:14:20]

bondd@chromium.org changed required reviewers:
+ dbeam@chromium.org

[bondd, 2014-10-29 21:14:21]

Ready for review.
This CL was split from https://codereview.chromium.org/664583006/
I responded to the old reviewer comments in that CL in case that's useful for
reviewing this one.

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/inline_editable_list.js (right):

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:37: focusedColumnIndex:
-1,
Is there a style recommendation for whether this should this go before or after
the private member variables?

[Dan Beam, 2014-10-29 22:39:07]

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/inline_editable_list.js (right):

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:37: focusedColumnIndex:
-1,
On 2014/10/29 21:14:21, bondd wrote:
> Is there a style recommendation for whether this should this go before or
after
> the private member variables?

why is it not private?  because it's edited by the list?

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:457: return
element.getAttribute('inlineeditable-column');
this returns a string, not a number

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:561:
this.focusedColumnIndex = target.getAttribute('inlineeditable-column');
this is not a number, it's a string

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:639: * @param {Event} e
The property change event.
don't duplicate doc if this is an @override

[bondd, 2014-10-29 23:45:25]

Addressed reviewer comments from dbeam@.

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/inline_editable_list.js (right):

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:37: focusedColumnIndex:
-1,
On 2014/10/29 22:39:07, Dan Beam wrote:
> On 2014/10/29 21:14:21, bondd wrote:
> > Is there a style recommendation for whether this should this go before or
> after
> > the private member variables?
> 
> why is it not private?  because it's edited by the list?

Yes, it's edited by the list to tell the item which column was focused in the
previously focused item. Item can't figure it out by itself because it can't
(shouldn't) look into its siblings to see which column they had focused.

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:457: return
element.getAttribute('inlineeditable-column');
On 2014/10/29 22:39:07, Dan Beam wrote:
> this returns a string, not a number

Done. Is there a better way to do this, maybe by storing the attribute as a
number somehow? This seems to be the recommended way to do things, but maybe I
am missing something.

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:561:
this.focusedColumnIndex = target.getAttribute('inlineeditable-column');
On 2014/10/29 22:39:07, Dan Beam wrote:
> this is not a number, it's a string

Done.

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:639: * @param {Event} e
The property change event.
On 2014/10/29 22:39:07, Dan Beam wrote:
> don't duplicate doc if this is an @override

Done.

[Dan Beam, 2014-10-30 19:34:45]

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/inline_editable_list.js (right):

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:457: return
element.getAttribute('inlineeditable-column');
On 2014/10/29 23:45:25, bondd wrote:
> On 2014/10/29 22:39:07, Dan Beam wrote:
> > this returns a string, not a number
> 
> Done. Is there a better way to do this, maybe by storing the attribute as a
> number somehow? This seems to be the recommended way to do things, but maybe I
> am missing something.

no, setAttribute() always stringifies.  you could save it in JS only (e.g. just
element.editColumn_) but you'd have to change the way that you querySelector()
for [inlineeditable-column] (which would probably be fine).

[bondd, 2014-10-30 20:34:06]

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/inline_editable_list.js (right):

https://codereview.chromium.org/688043003/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/inline_editable_list.js:457: return
element.getAttribute('inlineeditable-column');
On 2014/10/30 19:34:45, Dan Beam wrote:
> On 2014/10/29 23:45:25, bondd wrote:
> > On 2014/10/29 22:39:07, Dan Beam wrote:
> > > this returns a string, not a number
> > 
> > Done. Is there a better way to do this, maybe by storing the attribute as a
> > number somehow? This seems to be the recommended way to do things, but maybe
I
> > am missing something.
> 
> no, setAttribute() always stringifies.  you could save it in JS only (e.g.
just
> element.editColumn_) but you'd have to change the way that you querySelector()
> for [inlineeditable-column] (which would probably be fine).

So is it fine the way it is now in patch set #2 (with the parseInt call), or
would you like me to change this somehow?

[Dan Beam, 2014-10-30 22:29:11]

lgtm

[bondd, 2014-11-12 23:16:50]

The CQ bit was checked by bondd@chromium.org

[commit-bot: I haz the power, 2014-11-12 23:18:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/688043003/40001

[commit-bot: I haz the power, 2014-11-13 00:17:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-13 00:17:39]

Patchset 3 (id:??) landed as
https://crrev.com/39698bd5cf1a61e67796b9a855f32321a1d29242
Cr-Commit-Position: refs/heads/master@{#303933}