support common content-language mistakes

It seems that the improved recognition of the Content-Language information means that new web pages with incorrect language specification are being found.  This CL corrects some of the common errors so that the language specification is recognized by the translate helper.

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

[MAD, 2012-11-06 18:00:20]

LGTM with a few small requests...

Thanks for the quick fix!

BYE
MAD

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper.cc (right):

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:85: language[underscore_index] = '-';
No need for {} here...

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:86: }
Add a DCHECK that there are not other '_' in the string.

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:91: language =
StringToLowerASCII(language.substr(0, dash_index)) +
Add a DCHECK that there are no other '-' in the string

[bcwhite, 2012-11-06 18:37:48]

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper.cc (right):

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:85: language[underscore_index] = '-';
Doh!  I am going to do that _every_ time!
Fixed.

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:86: }
On 2012/11/06 18:00:20, MAD wrote:
> Add a DCHECK that there are not other '_' in the string.

We're parsing user-supplied input here.  Is crashing as a result of a web-page
really a good idea, even if only in debug mode?

The translation code strips everything after the first dash if it doesn't find a
match and tries again.

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:91: language =
StringToLowerASCII(language.substr(0, dash_index)) +
On 2012/11/06 18:00:20, MAD wrote:
> Add a DCHECK that there are no other '-' in the string

Same as above.  I don't think it's a problem as malformed input just leads to
malformed output and will either match only the base language or get ignored for
no match.

[MAD, 2012-11-06 18:39:30]

still lgtm...

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
File chrome/renderer/translate_helper.cc (right):

https://codereview.chromium.org/11358121/diff/1/chrome/renderer/translate_hel...
chrome/renderer/translate_helper.cc:86: }
On 2012/11/06 18:37:49, bcwhite wrote:
> On 2012/11/06 18:00:20, MAD wrote:
> > Add a DCHECK that there are not other '_' in the string.
> 
> We're parsing user-supplied input here.  Is crashing as a result of a web-page
> really a good idea, even if only in debug mode?
> 
> The translation code strips everything after the first dash if it doesn't find
a
> match and tries again.

OK, fine...

[bcwhite, 2012-11-06 18:42:27]

James, could I get an OWNERS review, please?

-- Brian

[James Hawkins, 2012-11-06 18:47:49]

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
chrome/renderer/translate_helper_browsertest.cc:398: // Tests that the language
meta tag is converted to chrome standard of dashes
re: chrome standard - Is this just Chrome's interpretation of the lang meta tag,
or are sites truly not following the definition of a web standard (I'm not that
familiar with this area of standards)?

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
chrome/renderer/translate_helper_browsertest.cc:401:
TEST_F(ChromeRenderViewTest, LanguageFormatCorrect) {
nit: Please use a more precise test name.

[bcwhite, 2012-11-06 18:55:23]

New CL uploaded.

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
File chrome/renderer/translate_helper_browsertest.cc (right):

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
chrome/renderer/translate_helper_browsertest.cc:398: // Tests that the language
meta tag is converted to chrome standard of dashes
> re: chrome standard - Is this just Chrome's interpretation of the lang meta
tag,
> or are sites truly not following the definition of a web standard (I'm not
that
> familiar with this area of standards)?

As far as I could find, dashes are the official way of defining the language. 
However, underscores are not uncommon, probably because Java uses underscores so
that they are valid identifiers in the language.  Thus, people are used to
seeing both and don't differentiate.

https://codereview.chromium.org/11358121/diff/6002/chrome/renderer/translate_...
chrome/renderer/translate_helper_browsertest.cc:401:
TEST_F(ChromeRenderViewTest, LanguageFormatCorrect) {
On 2012/11/06 18:47:49, James Hawkins wrote:
> nit: Please use a more precise test name.

Done.

[James Hawkins, 2012-11-06 19:30:20]

lgtm

[commit-bot: I haz the power, 2012-11-06 19:54:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bcwhite@chromium.org/11358121/12001

[commit-bot: I haz the power, 2012-11-06 21:58:10]

Change committed as 166276