Fixed failure to add language in language settings page.

Fixed failure to add language in language settings page.


BUG=125323
TEST=browser_tests --gtest_filter=WebUIBidiCheckerBrowserTestRTL.TestSettingsLanguageOptionsPage


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134427

[Danh Nguyen, 2012-04-27 21:22:41]

Hi Chris,

This CL fixes a P0 for M20.  Could you help to review it ASAP?

Thanks,
Danh

[csilv, 2012-04-27 21:29:48]

http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
File chrome/browser/resources/options2/language_list.js (right):

http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
chrome/browser/resources/options2/language_list.js:73:
this.languageCodeToLanguageInfo_[language.code] = languageList[i];
I'm confused by this change, can you explain it?

[Danh Nguyen, 2012-04-27 21:38:52]

Thanks Chris.  Please see my explanation below.

Danh

http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
File chrome/browser/resources/options2/language_list.js (right):

http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
chrome/browser/resources/options2/language_list.js:73:
this.languageCodeToLanguageInfo_[language.code] = languageList[i];
On 2012/04/27 21:29:48, csilv wrote:
> I'm confused by this change, can you explain it?

The parameter languageCode is a language code of the language to be looked up. 
It doesn't change in this function's scope.  Meanwhile, language.code is the
language code of the current language when we iterate the language list in the
for loop.  So it changes inside the loop.  Without this change, the mapping from
language code to language info will have only one entry, indexed by
languageCode, which is incorrect.

[Danh Nguyen, 2012-04-27 21:41:47]

I made an error while cleaning up the changes.  Please ignore my previous email.
 I'll upload a new patch soon.

Thanks,
Danh

On 2012/04/27 21:38:52, Danh Nguyen wrote:
> Thanks Chris.  Please see my explanation below.
> 
> Danh
> 
>
http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
> File chrome/browser/resources/options2/language_list.js (right):
> 
>
http://codereview.chromium.org/10255017/diff/1/chrome/browser/resources/optio...
> chrome/browser/resources/options2/language_list.js:73:
> this.languageCodeToLanguageInfo_[language.code] = languageList[i];
> On 2012/04/27 21:29:48, csilv wrote:
> > I'm confused by this change, can you explain it?
> 
> The parameter languageCode is a language code of the language to be looked up.

> It doesn't change in this function's scope.  Meanwhile, language.code is the
> language code of the current language when we iterate the language list in the
> for loop.  So it changes inside the loop.  Without this change, the mapping
from
> language code to language info will have only one entry, indexed by
> languageCode, which is incorrect.

[Danh Nguyen, 2012-04-27 23:30:44]

Hi Chris,

I've just got home and uploaded a new patch.  Could you take a quick look?

Thanks,
Danh

[csilv, 2012-04-27 23:49:30]

lgtm

[commit-bot: I haz the power, 2012-04-28 00:31:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danhn@chromium.org/10255017/10001

[commit-bot: I haz the power, 2012-04-28 02:20:31]

Change committed as 134427