Fixed button focusing in OOBE screens.

Fixed button focusing in OOBE screens.

Before focusing did not work, which was especially noticeable for users using ChromeVox or those using keyboard only. This patch resolves following issues:
* Automatically focus the default button (depending on the screen).
* Focus the cancel button by default in the reset screen.
Besides, now it is possible to focus any control by default, not only buttons.

TEST=Turn on ChromeVox and navigate through the oobe screens.
BUG=161896
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170524

[mtomasz, 2012-11-27 23:17:58]

@nkostylev: PTAL.

[Nikita (slow), 2012-11-28 18:17:36]

I've executed cros_* trybots which run CrOS smoke tests on virtual machines.

[Nikita (slow), 2012-11-28 18:26:26]

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/display_manager.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:175: var buttons =
document.querySelectorAll(
I think this should be implemented differently.

You know screen so you should screen.buttons property and check which of those
have autofocus set.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:179:
buttons[i].focus();
break;

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:264: }.bind(this));
Have you tested how OOBE works with this change?

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe.js:98:
Oobe.getInstance().restoreFocus();  // Restore the screen's focus.
nit: Please place comment above this line.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
(right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js:113:
cancelButton.autofocus = true;
Not sure that we would want to autofocus this button as focus is inside GAIA
sign in form that is shown on this screen.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_user_image.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_user_image.js:138:
okButton.autofocus = true;
Not sure that we want to auto focus Ok button here.
Originally focus is on user images list so user could use keyboard to select
those, then press Enter to confirm selection. Then press Enter one more time to
close screen.

[mtomasz, 2012-11-28 21:57:42]

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/display_manager.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:175: var buttons =
document.querySelectorAll(
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> I think this should be implemented differently.
> 
> You know screen so you should screen.buttons property and check which of those
> have autofocus set.

Isn't screen.buttons getter producing new buttons every time it is called? If
so, then I can't use it.

This method is just a copy paste of disableButtons_().

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:179:
buttons[i].focus();
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> break;

Done.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:264: }.bind(this));
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> Have you tested how OOBE works with this change?

Yes, tested locally.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe.js:98:
Oobe.getInstance().restoreFocus();  // Restore the screen's focus.
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> nit: Please place comment above this line.

Done.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
(right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js:113:
cancelButton.autofocus = true;
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> Not sure that we would want to autofocus this button as focus is inside GAIA
> sign in form that is shown on this screen.

Done. If we want it to be focused, then it will be done by showStep(). By the
way, can I somehow test this screen locally? It didn't work for me.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_user_image.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_user_image.js:138:
okButton.autofocus = true;
On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> Not sure that we want to auto focus Ok button here.
> Originally focus is on user images list so user could use keyboard to select
> those, then press Enter to confirm selection. Then press Enter one more time
to
> close screen.

Done.

[mtomasz, 2012-11-28 23:50:25]

Hi Nikita, I've rewritten it since I think that the previous approach wasn't
good. Now we can set a default focus on any control, which may be very more
useful. Also, I removed restoring focus when closing the popup from this CL.

[Nikita (slow), 2012-11-29 16:08:11]

Btw, I've tested your patch.
One thing that is regressed - TPM dialog on EULA could not be closed.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/display_manager.js (right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/display_manager.js:175: var buttons =
document.querySelectorAll(
On 2012/11/28 21:57:42, mtomasz wrote:
> On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> > I think this should be implemented differently.
> > 
> > You know screen so you should screen.buttons property and check which of
those
> > have autofocus set.
> 
> Isn't screen.buttons getter producing new buttons every time it is called? If
> so, then I can't use it.
> 
> This method is just a copy paste of disableButtons_().

You're right, screen.buttons always creates new elements so could not be used
here.

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
(right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js:113:
cancelButton.autofocus = true;
On 2012/11/28 21:57:42, mtomasz wrote:
> On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> > Not sure that we would want to autofocus this button as focus is inside GAIA
> > sign in form that is shown on this screen.
> 
> Done. If we want it to be focused, then it will be done by showStep(). By the
> way, can I somehow test this screen locally? It didn't work for me.

Did you try Ctrl+Alt+E shortcut.

You should launch Chrome in OOBE, go through first screens and when you're at
GAIA login, press Ctrl+Alt+E.
This shortcut also works on normal login screen with users for me.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe.js (left):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe.js:97: $('popup-overlay').hidden =
true;
Why did you remove this part? Dialog could not be closed now.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe_screen_eula.js (right):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_eula.js:73: * Returns a
control which should retrieve an initial focus.
retrieve > receive

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe_screen_network.js (right):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_network.js:84: * Returns a
control which should retrieve an initial focus.
retrieve > receive

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_network.js:87: return
$('continue-button');
When you boot into OOBE there's no network selected so continue button is
disabled. It makes sense to focus $('language-select')

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe_screen_reset.js (right):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_reset.js:71: * Returns a
control which should retrieve an initial focus.
retrieve > receive

[mtomasz, 2012-11-29 19:04:28]

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
(right):

https://codereview.chromium.org/11421111/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js:113:
cancelButton.autofocus = true;
On 2012/11/29 16:08:11, Nikita Kostylev wrote:
> On 2012/11/28 21:57:42, mtomasz wrote:
> > On 2012/11/28 18:26:26, Nikita Kostylev wrote:
> > > Not sure that we would want to autofocus this button as focus is inside
GAIA
> > > sign in form that is shown on this screen.
> > 
> > Done. If we want it to be focused, then it will be done by showStep(). By
the
> > way, can I somehow test this screen locally? It didn't work for me.
> 
> Did you try Ctrl+Alt+E shortcut.
> 
> You should launch Chrome in OOBE, go through first screens and when you're at
> GAIA login, press Ctrl+Alt+E.
> This shortcut also works on normal login screen with users for me.

I've tested and removed the change from this file since it was useless here.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe.js (left):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe.js:97: $('popup-overlay').hidden =
true;
On 2012/11/29 16:08:11, Nikita Kostylev wrote:
> Why did you remove this part? Dialog could not be closed now.

Done.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe_screen_network.js (right):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_network.js:84: * Returns a
control which should retrieve an initial focus.
On 2012/11/29 16:08:11, Nikita Kostylev wrote:
> retrieve > receive

Done.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_network.js:87: return
$('continue-button');
On 2012/11/29 16:08:11, Nikita Kostylev wrote:
> When you boot into OOBE there's no network selected so continue button is
> disabled. It makes sense to focus $('language-select')

sgtm.

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
File chrome/browser/resources/chromeos/login/oobe_screen_reset.js (right):

https://codereview.chromium.org/11421111/diff/11001/chrome/browser/resources/...
chrome/browser/resources/chromeos/login/oobe_screen_reset.js:71: * Returns a
control which should retrieve an initial focus.
On 2012/11/29 16:08:11, Nikita Kostylev wrote:
> retrieve > receive

Done.

[Nikita (slow), 2012-11-30 14:47:05]

lgtm

https://codereview.chromium.org/11421111/diff/2004/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_reset.js (right):

https://codereview.chromium.org/11421111/diff/2004/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_reset.js:60:
cancelButton.autofocus = true;
nit: is this still needed?

[Nikita (slow), 2012-11-30 14:47:26]

I've executed trybots.

[mtomasz, 2012-11-30 18:35:31]

https://codereview.chromium.org/11421111/diff/2004/chrome/browser/resources/c...
File chrome/browser/resources/chromeos/login/oobe_screen_reset.js (right):

https://codereview.chromium.org/11421111/diff/2004/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/oobe_screen_reset.js:60:
cancelButton.autofocus = true;
On 2012/11/30 14:47:05, Nikita Kostylev wrote:
> nit: is this still needed?

Done.

[commit-bot: I haz the power, 2012-11-30 18:36:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11421111/9002

[commit-bot: I haz the power, 2012-11-30 20:25:47]

Change committed as 170524