Add language and keyboard layout pickers to public session pods

Add language and keyboard layout pickers to public session pods

This CL adds language and keyboard pickers to public session pods. The
CL focuses on the necessary changes to the login UI. Follow-up CLs will:
- wire up the pickers so that they actually influence the language and
  keyboard layout in the public session, and
- customize the contents of the language picker based on policy
  (languages recommended by policy go first; the top recommended language
  is pre-selected)

BUG=214904, 241790
TEST=Manual, including RTL

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

[bartfab (slow), 2014-07-15 18:36:12]

Hi Pavel,

Could you take a look please?

FYI - This CL depends on:
https://codereview.chromium.org/397723002/

[dzhioev (left Google), 2014-07-16 18:42:29]

+alemate@ for l10n_util.cc review.

[Alexander Alekseev, 2014-07-16 19:35:51]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/l10n_util.cc (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:170:
scoped_ptr<base::ListValue> GetKeyboardLayouts(const std::string& locale) {
We have similar code for the Guest session:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

So, for the guest session we explicitly enable all hardware layouts plus locale
layouts.

Don't you think it is reasonable to enable all hardware layouts?

Also, I don't know if we have domain policy with default layout.
But if there is one, should we explicitly enable it and make default? (This
would be different against Guest session, because it is
owner/policy-independent.)

[bartfab (slow), 2014-07-21 14:49:14]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/l10n_util.cc (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:170:
scoped_ptr<base::ListValue> GetKeyboardLayouts(const std::string& locale) {
On 2014/07/16 19:35:50, alemate wrote:
> We have similar code for the Guest session:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
> 
> So, for the guest session we explicitly enable all hardware layouts plus
locale
> layouts.
> 
> Don't you think it is reasonable to enable all hardware layouts?

The difference is that the code we have for guest actually enables layouts. I do
not want to enable any layouts - I just want to enumerate them. I only want to
obtain a list that I can show in the UI. This way, if the user clicks away from
the public session pod without starting a session, the l10n settings will be
unaffected by the choices made in the pod.

> 
> Also, I don't know if we have domain policy with default layout.
> But if there is one, should we explicitly enable it and make default? (This
> would be different against Guest session, because it is
> owner/policy-independent.)

My work is all about adding a new policy to set the default layout. I just broke
it into several smaller CLs. This one adds a language picker that simply
defaults to the first language in the list. A follow-up CL will wire it up to
the policy.

[dzhioev (left Google), 2014-07-21 14:57:49]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
File chrome/browser/resources/login/user_pod_row.js (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1191: this.left =
$('pod-row').offsetWidth - rowPadding - width;
If we have a single public pod on screen, it is expanded to left and its right
edge is fixed. It looks strange. Is it expected behavior?

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1191: this.left =
$('pod-row').offsetWidth - rowPadding - width;
I can't find where you change 'left' back, when pod shrinks.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1205:
languageAndInputSection.addEventListener('webkitTransitionEnd',
Probably you want to add 'ensureTransitionEndEvent' call after each subscription
to 'webkitTransitionEnd' event, because sometimes 'webkitTransitionEnd' doesn't
fire.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
File chrome/browser/resources/login/user_pod_template.html (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_template.html:96: <select
class="language-select"></select>
I don't know why, but when there is single public pod on screen, language
selection closes when you release mouse button.

[bartfab (slow), 2014-07-21 16:50:04]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
File chrome/browser/resources/login/user_pod_row.js (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1191: this.left =
$('pod-row').offsetWidth - rowPadding - width;
On 2014/07/21 14:57:48, dzhioev wrote:
> If we have a single public pod on screen, it is expanded to left and its right
> edge is fixed. It looks strange. Is it expected behavior? 

I did not change the logic that makes space for expanded pods. It simply moved
from line 1007 of the file to here. And yes, space is always added on the left -
I did not make that decision, I just kept it as-is.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1191: this.left =
$('pod-row').offsetWidth - rowPadding - width;
On 2014/07/21 14:57:48, dzhioev wrote:
> I can't find where you change 'left' back, when pod shrinks.

I did not change the logic that preserves the pod's original position. |set
expanded(expanded)| stores the position in |this.usualLeft| when the pod expands
and restores it when the pod contracts.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1205:
languageAndInputSection.addEventListener('webkitTransitionEnd',
On 2014/07/21 14:57:48, dzhioev wrote:
> Probably you want to add 'ensureTransitionEndEvent' call after each
subscription
> to 'webkitTransitionEnd' event, because sometimes 'webkitTransitionEnd'
doesn't
> fire.

Ugh. Is that a blink bug or by design?

Either way, timers added.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
File chrome/browser/resources/login/user_pod_template.html (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_template.html:96: <select
class="language-select"></select>
On 2014/07/21 14:57:48, dzhioev wrote:
> I don't know why, but when there is single public pod on screen, language
> selection closes when you release mouse button.

Nice catch. Fix.

[Alexander Alekseev, 2014-07-21 17:06:56]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/l10n_util.cc (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:170:
scoped_ptr<base::ListValue> GetKeyboardLayouts(const std::string& locale) {
On 2014/07/21 14:49:13, bartfab wrote:
> On 2014/07/16 19:35:50, alemate wrote:
> > We have similar code for the Guest session:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
> > 
> > So, for the guest session we explicitly enable all hardware layouts plus
> locale
> > layouts.
> > 
> > Don't you think it is reasonable to enable all hardware layouts?
> 
> The difference is that the code we have for guest actually enables layouts. I
do
> not want to enable any layouts - I just want to enumerate them. I only want to
> obtain a list that I can show in the UI. This way, if the user clicks away
from
> the public session pod without starting a session, the l10n settings will be
> unaffected by the choices made in the pod.
> 
> > Also, I don't know if we have domain policy with default layout.
> > But if there is one, should we explicitly enable it and make default? (This
> > would be different against Guest session, because it is
> > owner/policy-independent.)
> 
> My work is all about adding a new policy to set the default layout. I just
broke
> it into several smaller CLs. This one adds a language picker that simply
> defaults to the first language in the list. A follow-up CL will wire it up to
> the policy.

Ah, I see what you are doing. Thank you for explanation.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:173:
std::vector<std::string> layouts = util->GetHardwareLoginInputMethodIds();
May be GetHardwareInputMethodIds ? (not "Login")
User doesn't need to immediately input login information, so it would be strange
if he couldn't select any hardware layout. What do you think?

[Nikita (slow), 2014-07-21 17:08:51]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
File chrome/browser/resources/login/user_pod_row.js (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/resources/log...
chrome/browser/resources/login/user_pod_row.js:1205:
languageAndInputSection.addEventListener('webkitTransitionEnd',
On 2014/07/21 16:50:04, bartfab wrote:
> On 2014/07/21 14:57:48, dzhioev wrote:
> > Probably you want to add 'ensureTransitionEndEvent' call after each
> subscription
> > to 'webkitTransitionEnd' event, because sometimes 'webkitTransitionEnd'
> doesn't
> > fire.
> 
> Ugh. Is that a blink bug or by design?
> 
> Either way, timers added.

This is by design.
https://developer.mozilla.org/en-US/docs/Web/Events/transitionend

[Alexander Alekseev, 2014-07-21 17:13:41]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/l10n_util.cc (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:170:
scoped_ptr<base::ListValue> GetKeyboardLayouts(const std::string& locale) {
GetKeyboardLayouts() seems to be too general. We already have a lot of
"Get*KeyboardLayouts" and most of them (now) have descriptive names. And
everyone working on input methods make mistakes using not yet renamed
"GetKeyboardLayouts" .

Maybe rename to GetPublicSessionKeyboardLayouts() ?

[dzhioev (left Google), 2014-07-21 17:37:17]

LGTM

[bartfab (slow), 2014-07-21 18:20:48]

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/l10n_util.cc (right):

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:170:
scoped_ptr<base::ListValue> GetKeyboardLayouts(const std::string& locale) {
On 2014/07/21 17:13:41, alemate wrote:
> GetKeyboardLayouts() seems to be too general. We already have a lot of
> "Get*KeyboardLayouts" and most of them (now) have descriptive names. And
> everyone working on input methods make mistakes using not yet renamed
> "GetKeyboardLayouts" .
> 
> Maybe rename to GetPublicSessionKeyboardLayouts() ?

I called it GetKeyboardLayoutsForLocale(). After all, it returns all layouts for
the given locale plus all hardware layouts. This seems fairly generic and useful
outside public sessions.

https://codereview.chromium.org/398543002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/l10n_util.cc:173:
std::vector<std::string> layouts = util->GetHardwareLoginInputMethodIds();
On 2014/07/21 17:06:56, alemate wrote:
> May be GetHardwareInputMethodIds ? (not "Login")
> User doesn't need to immediately input login information, so it would be
strange
> if he couldn't select any hardware layout. What do you think?

Nice catch. Done.

[Alexander Alekseev, 2014-07-21 18:33:23]

lgtm

[bartfab (slow), 2014-07-23 08:00:13]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-07-23 08:01:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/398543002/80001

[commit-bot: I haz the power, 2014-07-23 13:08:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-23 13:32:52]

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

[commit-bot: I haz the power, 2014-07-23 13:32:53]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bartfab (slow), 2014-07-23 13:33:50]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-07-23 13:35:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/398543002/80001

[commit-bot: I haz the power, 2014-07-23 14:04:20]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-23 14:33:09]

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

[commit-bot: I haz the power, 2014-07-23 14:33:10]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bartfab (slow), 2014-07-23 14:41:37]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-07-23 14:42:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/398543002/80001

[commit-bot: I haz the power, 2014-07-23 15:19:09]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-23 15:57:35]

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

[commit-bot: I haz the power, 2014-07-23 15:57:35]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bartfab (slow), 2014-07-23 16:33:17]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-07-23 16:34:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/398543002/80001

[commit-bot: I haz the power, 2014-07-23 17:47:28]

Change committed as 284983