Add Locale Request Support to Chrome Now

Add Locale Request Support to Chrome Now

BUG=164227
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252215

[skare_, 2014-02-19 19:06:28]

lgtm

[robliao, 2014-02-19 19:07:08]

vadimt: Please provide owner approval for this CL.

[rgustafson, 2014-02-19 19:18:14]

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:585: requestParameters +=
'&uiLocale=' + navigator.language;
https://developer.mozilla.org/en-US/docs/Web/API/NavigatorLanguage.language

This says it's based on acceptlanguage? Have you tested this? It might just be
out of date.

[robliao, 2014-02-19 19:26:47]

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:585: requestParameters +=
'&uiLocale=' + navigator.language;
Works for me.

accept-language:en-US,en;q=0.8,de;q=0.6,de-DE;q=0.4,zh-TW;q=0.2,zh;q=0.2,ko;q=0.2

navigator.language = "ko"

Current UI language = "ko"

On 2014/02/19 19:18:14, rgustafson wrote:
> https://developer.mozilla.org/en-US/docs/Web/API/NavigatorLanguage.language
> 
> This says it's based on acceptlanguage? Have you tested this? It might just be
> out of date.

[rgustafson, 2014-02-19 19:39:15]

lgtm

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:585: requestParameters +=
'&uiLocale=' + navigator.language;
On 2014/02/19 19:26:47, robliao wrote:
> Works for me.
> 
>
accept-language:en-US,en;q=0.8,de;q=0.6,de-DE;q=0.4,zh-TW;q=0.2,zh;q=0.2,ko;q=0.2
> 
> navigator.language = "ko"
> 
> Current UI language = "ko"
> 
> On 2014/02/19 19:18:14, rgustafson wrote:
> > https://developer.mozilla.org/en-US/docs/Web/API/NavigatorLanguage.language
> > 
> > This says it's based on acceptlanguage? Have you tested this? It might just
be
> > out of date.
> 

Do you know the actual standard that these strings follow is since apparently
the documentation isn't that great? From what I gather it's RFC 5646, but this
is important so we don't choke on parsing this serverside.

[robliao, 2014-02-19 19:49:19]

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/172733002/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:585: requestParameters +=
'&uiLocale=' + navigator.language;
A bit more context. The reason why this works is because we get no preferred
languages while getting navigator.language. Haven't dug into why.

This causes us to fall into getting the command line langauge

3:035> k20
ChildEBP RetAddr  
0018c094 11b9c660 content!content::RenderThreadImpl::GetLocale+0xb5
0018c120 1b3c70e6
content!content::RendererWebKitPlatformSupportImpl::defaultLocale+0x60
0018c168 1b3c6f2c blink_platform!WebCore::platformLanguage+0xf6
0018c19c 1b3c6c0d blink_platform!WebCore::userPreferredLanguages+0x9c
0018c1d4 165b36ab blink_platform!WebCore::defaultLanguage+0x4d
0018c1ec 1522a0b2 blink_web!WebCore::Navigator::language+0x2b
0018c228 1522a019
blink_web!WebCore::NavigatorV8Internal::languageAttributeGetter+0x72
0018c234 0f905534
blink_web!WebCore::NavigatorV8Internal::languageAttributeGetterCallback+0x19
0018c270 0f6c0f44 v8!v8::internal::PropertyCallbackArguments::Call+0x74

Sure enough, our command line has
CommandLine:  '"D:\src\out\Debug\chrome.exe" --type=renderer --lang=ko
--force-fieldtrials=[Field Trials] --extension-process
--enable-threaded-compositing --enable-delegated-renderer
--enable-deadline-scheduling --enable-software-compositing --channel=[Channel]
    
So who sets --lang?

That gets set up here:
const CommandLinePrefStore::StringSwitchToPreferenceMapEntry
    CommandLinePrefStore::string_switch_map_[] = {
      { switches::kLang, prefs::kApplicationLocale }

On 2014/02/19 19:39:16, rgustafson wrote:
> On 2014/02/19 19:26:47, robliao wrote:
> > Works for me.
> > 
> >
>
accept-language:en-US,en;q=0.8,de;q=0.6,de-DE;q=0.4,zh-TW;q=0.2,zh;q=0.2,ko;q=0.2
> > 
> > navigator.language = "ko"
> > 
> > Current UI language = "ko"
> > 
> > On 2014/02/19 19:18:14, rgustafson wrote:
> > >
https://developer.mozilla.org/en-US/docs/Web/API/NavigatorLanguage.language
> > > 
> > > This says it's based on acceptlanguage? Have you tested this? It might
just
> be
> > > out of date.
> > 
> 
> Do you know the actual standard that these strings follow is since apparently
> the documentation isn't that great? From what I gather it's RFC 5646, but this
> is important so we don't choke on parsing this serverside.

[robliao, 2014-02-19 19:50:04]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-02-19 19:51:52]

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

[commit-bot: I haz the power, 2014-02-19 19:51:52]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[vadimt, 2014-02-19 21:37:43]

lgtm

[commit-bot: I haz the power, 2014-02-19 21:44:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[robliao, 2014-02-19 21:44:27]

The CQ bit was unchecked by robliao@chromium.org

[robliao, 2014-02-19 22:12:08]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-02-19 22:52:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[commit-bot: I haz the power, 2014-02-20 00:56:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[commit-bot: I haz the power, 2014-02-20 04:46:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[commit-bot: I haz the power, 2014-02-20 08:49:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[commit-bot: I haz the power, 2014-02-20 12:21:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/172733002/1

[commit-bot: I haz the power, 2014-02-20 13:25:04]

Change committed as 252215