Allow country-specific languages in UI list at OOBE.

Allow country-specific languages in UI list at OOBE.

This CL allows displaying to user and selection of country-specific language
codes for which we do not have actual translation. A base language locale
will be used instead.

BUG=357481
TEST=browsertest

To test manually:

1) Set VPD value of initial_locale to country-specific language codes, for which only the translation to base language exist. For example:
"fr-CH,it-CH,de-CH"

2) Start OOBE and check they are displayed and selectable like any other languages.

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

[Alexander Alekseev, 2014-04-03 23:36:22]

Please review:

jshin@: ui/base/l10n/*
dpoluknon@: customization_document
nkostylev@: all

[jungshik at Google, 2014-04-04 08:27:08]

lgtm

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:272:
language_id, base_language_code, true);
You can just use |language_id| instead of |base_language_code| as the 2nd param.
GetDisplayNameForLocale will do the necessary fallback. In most cases, it'd not
matter, but in some cases, there might be a difference.

[jungshik at Google, 2014-04-04 08:37:21]

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:76: const
std::string& loaded_locale) {
JFYI: It's also possible to ICU's Locale class (getLanguage()) here [1] , but  I
think what's necessary here is simple enough so that we don't need to bother. 

[1]
http://icu-project.org/apiref/icu4c/classicu_1_1Locale.html#af36d821adced72a8...

[Nikita (slow), 2014-04-04 15:32:37]

The CQ bit was checked by nkostylev@chromium.org

[commit-bot: I haz the power, 2014-04-04 15:32:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/224093013/20001

[commit-bot: I haz the power, 2014-04-04 15:33:00]

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

[commit-bot: I haz the power, 2014-04-04 15:33:00]

Failed to apply patch for
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
  Hunk #2 FAILED at 267.
  Hunk #3 succeeded at 400 (offset -9 lines).
  Hunk #4 succeeded at 441 (offset -9 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc.rej

Patch:       chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
Index: chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
b/chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
index
30d2e1f02211925f6037bda19cd6eda618f96653..80eed45e917c504bd7b40cf26eb47bda4bbdcd7b
100644
--- a/chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc
@@ -72,6 +72,32 @@ void AddOptgroupOtherLayouts(base::ListValue*
input_methods_list) {
   input_methods_list->Append(optgroup);
 }
 
+std::string CalculateSelectedLanguage(const std::string& requested_locale,
+                                      const std::string& loaded_locale) {
+  const size_t dash_pos_requested = requested_locale.find_first_of('-');
+
+  // Requested language doesn't have specific country.
+  if (dash_pos_requested == std::string::npos || dash_pos_requested == 0)
+    return loaded_locale;
+
+  const size_t dash_pos_loaded = loaded_locale.find_first_of('-');
+
+  // loaded_locale doesn't have 'country' name and requested_locale
+  // is a country-specific language of loaded_locale.
+  if (dash_pos_loaded == std::string::npos || dash_pos_loaded == 0) {
+    if (requested_locale.substr(0, dash_pos_requested) == loaded_locale)
+      return requested_locale;
+    // languages differ
+    return loaded_locale;
+  }
+  // Both locales are country-specific. Check that languages are the same.
+  if (requested_locale.substr(0, dash_pos_requested) ==
+      loaded_locale.substr(0, dash_pos_loaded))
+    return requested_locale;
+
+  return loaded_locale;
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -241,14 +267,25 @@ struct NetworkScreenHandlerOnLanguageChangedCallbackData {
 // static
 void NetworkScreenHandler::OnLanguageChangedCallback(
     scoped_ptr<NetworkScreenHandlerOnLanguageChangedCallbackData> context,
-    const std::string& /*requested locale*/,
-    const std::string& /*loaded_locale*/,
-    const bool /*success*/) {
+    const std::string& requested_locale,
+    const std::string& loaded_locale,
+    const bool success) {
   if (!context or !context->handler_)
     return;
 
   NetworkScreenHandler* const self = context->handler_.get();
 
+  if (success) {
+    if (requested_locale == loaded_locale) {
+      self->selected_language_code_ = requested_locale;
+    } else {
+      self->selected_language_code_ =
+          CalculateSelectedLanguage(requested_locale, loaded_locale);
+    }
+  } else {
+    self->selected_language_code_ = loaded_locale;
+  }
+
   base::DictionaryValue localized_strings;
   static_cast<OobeUI*>(self->web_ui()->GetController())
       ->GetLocalizedStrings(&localized_strings);
@@ -383,7 +420,6 @@ bool NetworkScreenHandler::IsDerelict() {
   return time_on_oobe_ >= derelict_detection_timeout_;
 }
 
-// static
 base::ListValue* NetworkScreenHandler::GetLanguageList() {
   const std::string app_locale = g_browser_process->GetApplicationLocale();
   input_method::InputMethodManager* manager =
@@ -425,7 +461,13 @@ base::ListValue* NetworkScreenHandler::GetLanguageList() {
 
     language_info->SetString("value", value);
     language_info->SetString("title", display_name);
-    language_info->SetBoolean("selected", value == app_locale);
+    if (selected_language_code_.empty()) {
+      if (value == app_locale)
+        language_info->SetBoolean("selected", true);
+    } else {
+      if (value == selected_language_code_)
+        language_info->SetBoolean("selected", true);
+    }
   }
   return languages_list;
 }

[Nikita (slow), 2014-04-04 15:33:29]

On 2014/04/04 15:32:37, Nikita Kostylev wrote:
> The CQ bit was checked by mailto:nkostylev@chromium.org

Sorry for that, wrong CL.

[Nikita (slow), 2014-04-04 15:34:17]

Can you please rebase CL first.

[Alexander Alekseev, 2014-04-05 01:18:32]

nkostylev@, dpolukhin@ please review.

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/224093013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:272:
language_id, base_language_code, true);
On 2014/04/04 08:27:08, Jungshik Shin wrote:
> You can just use |language_id| instead of |base_language_code| as the 2nd
param.
> GetDisplayNameForLocale will do the necessary fallback. In most cases, it'd
not
> matter, but in some cases, there might be a difference. 

Done.

[Nikita (slow), 2014-04-07 04:54:27]

Please add tests.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/customization_document.cc (right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/customization_document.cc:311:
std::for_each(configured_locales_.begin(),
Can you please add a comment with an example of what kind of mapping is
happening here?

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:75: std::string
CalculateSelectedLanguage(const std::string& requested_locale,
nit: Add comment.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:79: //
Requested language doesn't have specific country.
nit: It's better to use "region" instead of "country". Consider es-419.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:90: //
languages differ
nit: Capitalize comment.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:240:
// base_language_codes. If there are unsipported language variants,
nit: unsupported

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:242:
for (std::map<std::string, int>::const_iterator iter = language_index.begin();
Why this new code in chrome://settings is necessary?

I though that this change should only cover out-of-box first screen i.e. mostly
how locales are presented to the user based on VPD values since values stored in
app_locale pref will be mapped ones.

[Alexander Alekseev, 2014-04-07 16:56:28]

Please review.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/customization_document.cc (right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/customization_document.cc:311:
std::for_each(configured_locales_.begin(),
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> Can you please add a comment with an example of what kind of mapping is
> happening here?

Done.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:75: std::string
CalculateSelectedLanguage(const std::string& requested_locale,
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> nit: Add comment.

Done.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:79: //
Requested language doesn't have specific country.
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> nit: It's better to use "region" instead of "country". Consider es-419.

Done.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:90: //
languages differ
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> nit: Capitalize comment.

Done.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:240:
// base_language_codes. If there are unsipported language variants,
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> nit: unsupported

Done.

https://codereview.chromium.org/224093013/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:242:
for (std::map<std::string, int>::const_iterator iter = language_index.begin();
On 2014/04/07 04:54:27, Nikita Kostylev wrote:
> Why this new code in chrome://settings is necessary?
> 
> I though that this change should only cover out-of-box first screen i.e.
mostly
> how locales are presented to the user based on VPD values since values stored
in
> app_locale pref will be mapped ones.

Because the list of languages at OOBE is actually created here.

[Nikita (slow), 2014-04-07 17:06:58]

lgtm

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:153:
HandleOnLanguageChanged(startup_manifest->initial_locale_default());
This won't quit work. This screen may be shown multiple times if user changes
locale then goes to EULA and back to this screen.

Initial locale should be set just once, before out-of-box UI is even
initialized, that's happening in ShowLoginWizard() method.

[Alexander Alekseev, 2014-04-07 19:28:58]

jshin@, nkostylev@

Please review again.

We need to use CheckAndResolve locale because of en_AU case.

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:153:
HandleOnLanguageChanged(startup_manifest->initial_locale_default());
On 2014/04/07 17:06:58, Nikita Kostylev wrote:
> This won't quit work. This screen may be shown multiple times if user changes
> locale then goes to EULA and back to this screen.
> 
> Initial locale should be set just once, before out-of-box UI is even
> initialized, that's happening in ShowLoginWizard() method.

Done.

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/224093013/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:260:
const std::string base_language_code = language_id.substr(0, dash_pos);
This doesn't work for en_AU, because fallback is en_GB.
So I used CheckAndResolveLocale here.

[Nikita (slow), 2014-04-08 07:56:01]

lgtm

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:141: if
(selected_language_code_.empty()) {
What happens for this scenario:

1. First boot, out-of-box starts
2. Initial locale gets applied
3. User changes it to another locales
4. Proceeds to EULA and back (i.e. doesn't accept EULA).
5. Reboot.

So technically out-of-box hasn't been finished, even EULA hasn't been accepted
but user has changed locale.

Does this mean that on the next boot to network screen initial_locale will be
set again?
I'm not quite sure that this scenario works correctly now either so just
checking.

[Alexander Alekseev, 2014-04-08 11:58:18]

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:141: if
(selected_language_code_.empty()) {
On 2014/04/08 07:56:01, Nikita Kostylev wrote:
> What happens for this scenario:
> 
> 1. First boot, out-of-box starts
> 2. Initial locale gets applied
> 3. User changes it to another locales
> 4. Proceeds to EULA and back (i.e. doesn't accept EULA).
> 5. Reboot.
> 
> So technically out-of-box hasn't been finished, even EULA hasn't been accepted
> but user has changed locale.
> 
> Does this mean that on the next boot to network screen initial_locale will be
> set again?

Yes. Now each time network screen is shown after reboot, language will be set to
device default. I don't see easy way to fix it. We would need to store another
value in Local State to keep the initial user choice. I think that the
implemented behavior is less confusing.

> I'm not quite sure that this scenario works correctly now either so just
> checking.

[Nikita (slow), 2014-04-08 12:19:25]

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

https://codereview.chromium.org/224093013/diff/120001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:141: if
(selected_language_code_.empty()) {
On 2014/04/08 11:58:19, alemate wrote:
> On 2014/04/08 07:56:01, Nikita Kostylev wrote:
> > What happens for this scenario:
> > 
> > 1. First boot, out-of-box starts
> > 2. Initial locale gets applied
> > 3. User changes it to another locales
> > 4. Proceeds to EULA and back (i.e. doesn't accept EULA).
> > 5. Reboot.
> > 
> > So technically out-of-box hasn't been finished, even EULA hasn't been
accepted
> > but user has changed locale.
> > 
> > Does this mean that on the next boot to network screen initial_locale will
be
> > set again?
> 
> Yes. Now each time network screen is shown after reboot, language will be set
to
> device default. I don't see easy way to fix it. We would need to store another
> value in Local State to keep the initial user choice. I think that the
> implemented behavior is less confusing.
> 
> > I'm not quite sure that this scenario works correctly now either so just
> > checking.
> 

Ok then.

[Dmitry Polukhin, 2014-04-08 16:10:39]

lgtm

[Alexander Alekseev, 2014-04-09 17:33:29]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2014-04-09 17:34:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/224093013/120001

[commit-bot: I haz the power, 2014-04-09 21:01:42]

Change committed as 262808