fix target language issue under three locales (Hebrew[iw->he], Fillipion[tl->fil], and Nowreign[no-…

fix target language issue under three locales (Hebrew[iw->he], Fillipion[tl->fil], and Nowreign[no->nb]

BUG=664791
Review-Url: https://codereview.chromium.org/2802613004
Cr-Commit-Position: refs/heads/master@{#464758}
Committed: https://chromium.googlesource.com/chromium/src/+/5d482d74629b9a30493f0c0201fd6863ec1c1ece

[ftang1, 2017-04-06 00:14:34]

The CQ bit was checked by ftang@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-06 00:15:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ftang1, 2017-04-06 00:15:50]

ftang@google.com changed reviewers:
+ groby@chromium.org, jshin@chromium.org

[ftang1, 2017-04-06 00:15:51]

Please help code review.

[commit-bot: I haz the power, 2017-04-06 01:44:04]

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

[commit-bot: I haz the power, 2017-04-06 01:44:05]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-04-11 18:45:32]

LGTM, but would you add a TODO comment that this should be removed once Google
Translate API does the 'right' thing?

https://codereview.chromium.org/2802613004/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_manager.cc (right):

https://codereview.chromium.org/2802613004/diff/1/components/translate/core/b...
components/translate/core/browser/translate_manager.cc:518:
translate::ToTranslateLanguageSynonym(&language);
Shouldn't Google Translate API take care of this instead of clients like Chrome
doing this on their end?  

That is, Google Translate API should accept both {he,iw}, {tl, fil}, {nb, no}
and treat them as synonyms?

[ftang1, 2017-04-13 17:03:36]

On 2017/04/11 18:45:32, jungshik at Google wrote:
> LGTM, but would you add a TODO comment that this should be removed once Google
> Translate API does the 'right' thing?

Currently both side are doing "right thing".

> 
>
https://codereview.chromium.org/2802613004/diff/1/components/translate/core/b...
> File components/translate/core/browser/translate_manager.cc (right):
> 
>
https://codereview.chromium.org/2802613004/diff/1/components/translate/core/b...
> components/translate/core/browser/translate_manager.cc:518:
> translate::ToTranslateLanguageSynonym(&language);
> Shouldn't Google Translate API take care of this instead of clients like
Chrome
> doing this on their end?  
> 
> That is, Google Translate API should accept both {he,iw}, {tl, fil}, {nb, no}
> and treat them as synonyms?

The issue is Google Translate API return a list of supporting languages, which
does not include he, fil, nb in the list but include iw, tl, no in the list. And
in here the Chrome code decide what TARGET language should be based on the
Application locale matching one of the item in the list or not. And once that
got decided, the target language is put into the "Context menu" to show to the
user and the bubble to show to the user. So all these are done BEFORE requesting
the translation. So does Google Translate API also accept he, fil, nb in
translation request is irrelevant.

[ftang1, 2017-04-13 17:03:41]

The CQ bit was checked by ftang@google.com

[commit-bot: I haz the power, 2017-04-13 17:04:15]

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-04-13 17:14:46]

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

[commit-bot: I haz the power, 2017-04-13 17:14:47]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ftang1, 2017-04-13 17:41:08]

groby- need owner approval. Could you review it?

[groby-ooo-7-16, 2017-04-14 16:31:47]

lgtm

[ftang1, 2017-04-14 17:41:21]

The CQ bit was checked by ftang@google.com

[commit-bot: I haz the power, 2017-04-14 17:41:36]

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-04-14 18:26:53]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492191681894920, "parent_rev":
"449f505534de7c356cb86120a7da515199c5c337", "commit_rev":
"5d482d74629b9a30493f0c0201fd6863ec1c1ece"}

[commit-bot: I haz the power, 2017-04-14 18:27:45]

Description was changed from

==========
fix target language issue under three locales (Hebrew[iw->he],
Fillipion[tl->fil], and Nowreign[no->nb]

BUG=664791
==========

to

==========
fix target language issue under three locales (Hebrew[iw->he],
Fillipion[tl->fil], and Nowreign[no->nb]

BUG=664791

Review-Url: https://codereview.chromium.org/2802613004
Cr-Commit-Position: refs/heads/master@{#464758}
Committed:
https://chromium.googlesource.com/chromium/src/+/5d482d74629b9a30493f0c0201fd...
==========

[commit-bot: I haz the power, 2017-04-14 18:27:46]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/5d482d74629b9a30493f0c0201fd...