Fix tips in OS X to respect system language settings instead of locale....

Fix tips in OS X to respect system language settings instead of locale.

BUG= http://crbug.com/22727
TEST= Set preferred language to a different setting from the locale in the OS X system settings.
Make sure that the language of the tips matches the language setting (which
should be the same as that of the Chrome UI).  If tips are not available in
the Chrome UI language, no tips should be shown.


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

[Nico, 2009-10-06 22:39:00]

TEST= should really be "set preferred language and locale to something
different, make tip language matches preferred language (which is also used for
the rest of the UI)".

Other than that, LGTM with nit.

http://codereview.chromium.org/258037/diff/7001/8001
File chrome/browser/dom_ui/tips_handler.cc (right):

http://codereview.chromium.org/258037/diff/7001/8001#newcode47
Line 47: std::wstring locale = WebResourceService::GetWebResourceLanguage(
Rename the variable to something else too? At least on OS X, this is no longer a
locale in the strict sense. Also in the other places.

http://codereview.chromium.org/258037/diff/7001/8002
File chrome/browser/web_resource/web_resource_service.cc (right):

http://codereview.chromium.org/258037/diff/7001/8002#newcode305
Line 305: std::wstring localeList = prefs->GetString(prefs::kAcceptLanguages);
Looks like prefs::kAcceptLanguages is filled in from IDS_ACCEPT_LANGUAGES which
is hardcoded in every translation? Sneaky.

[Miranda Callahan, 2009-10-06 22:51:40]

Thank you!

http://codereview.chromium.org/258037/diff/7001/8001
File chrome/browser/dom_ui/tips_handler.cc (right):

http://codereview.chromium.org/258037/diff/7001/8001#newcode47
Line 47: std::wstring locale = WebResourceService::GetWebResourceLanguage(
On 2009/10/06 22:39:00, Nico wrote:
> Rename the variable to something else too? At least on OS X, this is no longer
a
> locale in the strict sense. Also in the other places.

Done.

[Nico, 2009-10-06 22:56:39]

http://codereview.chromium.org/258037/diff/7003/8004
File chrome/browser/dom_ui/tips_handler.cc (right):

http://codereview.chromium.org/258037/diff/7003/8004#newcode48
Line 48: current_prefs);
Here too.

[Miranda Callahan, 2009-10-06 23:00:55]

http://codereview.chromium.org/258037/diff/7003/8004
File chrome/browser/dom_ui/tips_handler.cc (right):

http://codereview.chromium.org/258037/diff/7003/8004#newcode48
Line 48: current_prefs);
On 2009/10/06 22:56:39, Nico wrote:
> Here too.

Whoops -- fixed on my machine, and forgot to hit flower-S before uploading. 
Still getting used to xcode, sorry about that.

[TVL, 2009-10-07 12:02:47]

http://codereview.chromium.org/258037/diff/7008/8009
File chrome/browser/web_resource/web_resource_service.cc (right):

http://codereview.chromium.org/258037/diff/7008/8009#newcode311
Line 311: #endif
drive by: since we're forking what is done here, I'm wondering what other places
call GetApplicationLocale and will be wrong?  ie-should we be looking at these
lower levels to change what the mac is returning so make sure the expectations
are right in all places?  and/or should the apis in question get comments added
about what they return why it differs on platforms.

[Nico, 2009-10-07 14:56:21]

http://codereview.chromium.org/258037/diff/7008/8009
File chrome/browser/web_resource/web_resource_service.cc (right):

http://codereview.chromium.org/258037/diff/7008/8009#newcode302
Line 302: #if defined OS_MACOSX
On second thought, shouldn't the os x version of this work on all platforms?

[Miranda Callahan, 2009-10-07 16:22:00]

http://codereview.chromium.org/258037/diff/7008/8009
File chrome/browser/web_resource/web_resource_service.cc (right):

http://codereview.chromium.org/258037/diff/7008/8009#newcode302
Line 302: #if defined OS_MACOSX
Well, the tips need to always be in the same language as the Chrome UI, but in
the Windows version, the preferred languages and the UI language are two
different settings (unlike the Mac side, which is, as you noticed, hard-coded to
values determined by the system language) -- so if we use the strategy for
Windows that we are using for the Mac side, we could, again, have the tips in a
different language from the UI.

One way to unify these two might be to have the Chrome UI language always be the
most preferred language on the Windows side, just as it is on the Mac side --
but I don't know if that makes sense for Windows.

On 2009/10/07 14:56:21, Nico wrote:
> On second thought, shouldn't the os x version of this work on all platforms?

http://codereview.chromium.org/258037/diff/7008/8009#newcode311
Line 311: #endif
Good point. I think a source of confusion is that on Windows,
GetApplicationLocale returns whatever the user sets as their Chrome UI Language
-- but on Mac, it returns the [NSLocale currentLocale], which can be set to be
different from the Chrome UI language.  

Perhaps there should be two different calls in l10n_util -- GetApplicationLocale
and GetApplicationLanguage; on the Windows side, they would both call the same
code (for now?), but on the Mac side, they would call the two different system
settings.  This way we could tease apart the two different meanings of "Locale"
in Mac, while allowing them to remain conflated in Windows. What do you think?

On 2009/10/07 12:02:48, TVL wrote:
> drive by: since we're forking what is done here, I'm wondering what other
places
> call GetApplicationLocale and will be wrong?  ie-should we be looking at these
> lower levels to change what the mac is returning so make sure the expectations
> are right in all places?  and/or should the apis in question get comments
added
> about what they return why it differs on platforms.