Fix behaviors of Add and Remove buttons.

Fix behaviors of Add and Remove buttons.

When a languge is added, select the newly added language.
When a language is removed, keep the same position if possible.

Along the way, remove unnecesary code from removeSelectedLanguage().

TEST=manually
BUG=chromium-os:4573
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55027

[mazda, 2010-08-04 10:32:05]

lgtm

[arv (Not doing code reviews), 2010-08-04 17:19:54]

FYI

http://codereview.chromium.org/2878079/diff/1/2
File chrome/browser/resources/options/chromeos_language_list.js (right):

http://codereview.chromium.org/2878079/diff/1/2#newcode67
chrome/browser/resources/options/chromeos_language_list.js:67:
this.selectionModel.selectedIndex = this.dataModel.length - 1;
You might want to set leadIndex and anchorIndex as well or keyboard navigation
will act as the user expects.

[satorux1, 2010-08-05 04:12:50]

http://codereview.chromium.org/2878079/diff/1/2
File chrome/browser/resources/options/chromeos_language_list.js (right):

http://codereview.chromium.org/2878079/diff/1/2#newcode67
chrome/browser/resources/options/chromeos_language_list.js:67:
this.selectionModel.selectedIndex = this.dataModel.length - 1;
On 2010/08/04 17:19:54, arv wrote:
> You might want to set leadIndex and anchorIndex as well or keyboard navigation
> will act as the user expects.

Thank you for pointing this out. We'll be supposedly switching to a single
selection model in the options page, so I would not add code for leadIndex and
anchorIndex here for now.

[arv1, 2010-08-05 17:01:48]

http://codereview.chromium.org/2884064/show

On Aug 4, 2010 9:12:50 PM PDT, satorux <satorux@chromium.org> wrote:
>> http://codereview.chromium.org/2878079/diff/1/2
>> File chrome/browser/resources/options/chromeos_language_list.js (right):
>> http://codereview.chromium.org/2878079/diff/1/2#newcode67
>> chrome/browser/resources/options/chromeos_language_list.js:67:
>> this.selectionModel.selectedIndex = this.dataModel.length - 1;
>> On 2010/08/04 17:19:54, arv wrote:
>> You might want to set leadIndex and anchorIndex as well or keyboard
>> navigation
>> will act as the user expects.

> Thank you for pointing this out. We'll be supposedly switching to a
> single selection model in the options page, so I would not add code for
> leadIndex and anchorIndex here for now.

[satorux1, 2010-08-06 00:39:35]

On 2010/08/05 17:01:48, arv1 wrote:
> http://codereview.chromium.org/2884064/show

Awesome! Please check it in so I can use it. :)


> 
> On Aug 4, 2010 9:12:50 PM PDT, satorux <mailto:satorux@chromium.org> wrote:
> >> http://codereview.chromium.org/2878079/diff/1/2
> >> File chrome/browser/resources/options/chromeos_language_list.js (right):
> >> http://codereview.chromium.org/2878079/diff/1/2#newcode67
> >> chrome/browser/resources/options/chromeos_language_list.js:67:
> >> this.selectionModel.selectedIndex = this.dataModel.length - 1;
> >> On 2010/08/04 17:19:54, arv wrote:
> >> You might want to set leadIndex and anchorIndex as well or keyboard
> >> navigation
> >> will act as the user expects.
> 
> > Thank you for pointing this out. We'll be supposedly switching to a
> > single selection model in the options page, so I would not add code for
> > leadIndex and anchorIndex here for now.
>

[arv (Not doing code reviews), 2010-08-06 00:52:39]

Checked in but it needs a WebKit roll.

erik



On Thu, Aug 5, 2010 at 17:39,  <satorux@chromium.org> wrote:
> On 2010/08/05 17:01:48, arv1 wrote:
>>
>> http://codereview.chromium.org/2884064/show
>
> Awesome! Please check it in so I can use it. :)
>
>
>
>> On Aug 4, 2010 9:12:50 PM PDT, satorux <mailto:satorux@chromium.org>
>> wrote:
>> >> http://codereview.chromium.org/2878079/diff/1/2
>> >> File chrome/browser/resources/options/chromeos_language_list.js
>> >> (right):
>> >> http://codereview.chromium.org/2878079/diff/1/2#newcode67
>> >> chrome/browser/resources/options/chromeos_language_list.js:67:
>> >> this.selectionModel.selectedIndex = this.dataModel.length - 1;
>> >> On 2010/08/04 17:19:54, arv wrote:
>> >> You might want to set leadIndex and anchorIndex as well or keyboard
>> >> navigation
>> >> will act as the user expects.
>
>> > Thank you for pointing this out. We'll be supposedly switching to a
>> > single selection model in the options page, so I would not add code for
>> > leadIndex and anchorIndex here for now.
>
>
>
>
> http://codereview.chromium.org/2878079/show
>

[satorux1, 2010-08-06 01:00:19]

On Fri, Aug 6, 2010 at 9:52 AM, Erik Arvidsson <arv@chromium.org> wrote:

> Checked in but it needs a WebKit roll.


Thanks. Which WebKit change? I'm looking forward to it.

Satoru



> erik



> On Thu, Aug 5, 2010 at 17:39,  <satorux@chromium.org> wrote:
> > On 2010/08/05 17:01:48, arv1 wrote:
> >>
> >> http://codereview.chromium.org/2884064/show
> >
> > Awesome! Please check it in so I can use it. :)
> >
> >
> >
> >> On Aug 4, 2010 9:12:50 PM PDT, satorux <mailto:satorux@chromium.org>
> >> wrote:
> >> >> http://codereview.chromium.org/2878079/diff/1/2
> >> >> File chrome/browser/resources/options/chromeos_language_list.js
> >> >> (right):
> >> >> http://codereview.chromium.org/2878079/diff/1/2#newcode67
> >> >> chrome/browser/resources/options/chromeos_language_list.js:67:
> >> >> this.selectionModel.selectedIndex = this.dataModel.length - 1;
> >> >> On 2010/08/04 17:19:54, arv wrote:
> >> >> You might want to set leadIndex and anchorIndex as well or keyboard
> >> >> navigation
> >> >> will act as the user expects.
> >
> >> > Thank you for pointing this out. We'll be supposedly switching to a
> >> > single selection model in the options page, so I would not add code
> for
> >> > leadIndex and anchorIndex here for now.
> >
> >
> >
> >
> > http://codereview.chromium.org/2878079/show
> >

[arv (Not doing code reviews), 2010-08-06 05:48:13]

Sorry, I had another code review that was depending on a webkit change
and I got confused.

The chromium CL is not yet submitted. I'll submit it tomorrow morning.

erik



On Thu, Aug 5, 2010 at 17:59, Satoru Takabayashi <satorux@chromium.org> wrote:
>
>
> On Fri, Aug 6, 2010 at 9:52 AM, Erik Arvidsson <arv@chromium.org> wrote:
>>
>> Checked in but it needs a WebKit roll.
>
> Thanks. Which WebKit change? I'm looking forward to it.
> Satoru
>
>>
>> erik
>>
>>
>>
>> On Thu, Aug 5, 2010 at 17:39,  <satorux@chromium.org> wrote:
>> > On 2010/08/05 17:01:48, arv1 wrote:
>> >>
>> >> http://codereview.chromium.org/2884064/show
>> >
>> > Awesome! Please check it in so I can use it. :)
>> >
>> >
>> >
>> >> On Aug 4, 2010 9:12:50 PM PDT, satorux <mailto:satorux@chromium.org>
>> >> wrote:
>> >> >> http://codereview.chromium.org/2878079/diff/1/2
>> >> >> File chrome/browser/resources/options/chromeos_language_list.js
>> >> >> (right):
>> >> >> http://codereview.chromium.org/2878079/diff/1/2#newcode67
>> >> >> chrome/browser/resources/options/chromeos_language_list.js:67:
>> >> >> this.selectionModel.selectedIndex = this.dataModel.length - 1;
>> >> >> On 2010/08/04 17:19:54, arv wrote:
>> >> >> You might want to set leadIndex and anchorIndex as well or keyboard
>> >> >> navigation
>> >> >> will act as the user expects.
>> >
>> >> > Thank you for pointing this out. We'll be supposedly switching to a
>> >> > single selection model in the options page, so I would not add code
>> >> > for
>> >> > leadIndex and anchorIndex here for now.
>> >
>> >
>> >
>> >
>> > http://codereview.chromium.org/2878079/show
>> >
>
>