Fix to enable toggling accessibility using Ctrl+Alt+Z

Fix to enable toggling accessibility using Ctrl+Alt+Z

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

bug=chromium-os:9414

[xiyuan, 2010-12-15 22:44:52]

This is for http://crosbug.com/9414, right?

FYI, in your commit message/issue description, you could put:
BUG=chromium-os:9414 for relevant bug.

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/wizard_accessibility_handler.cc (right):

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/wizard_accessibility_handler.cc:123: }
/tmp/mydebug.txt is debugging code, right? If so, please clean up.

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/wizard_accessibility_handler.cc:137: }
please clean up the debug code.

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/wizard_accessibility_helper.cc (right):

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/wizard_accessibility_helper.cc:29: //      new
views::Accelerator(app::VKEY_Z, false, true, true)
please remove the TODO since you re-enabled the hotkey.

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/wizard_accessibility_helper.cc:135: "Accessibility
turned off",
Do we need to move the string literals into resources grd?

Out of curiosity, would i18n be supported?

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/wizard_controller.cc:129: if (accel ==
accel_enable_accessibility_) {
nit: accel_enable_accessibility_ -> accel_toggle_accessibility_ since the hotkey
is toggling instead of enabling now.

[Chaitanya, 2010-12-15 22:59:49]

On 2010/12/15 22:44:52, xiyuan wrote:
> This is for http://crosbug.com/9414, right?
> 
> FYI, in your commit message/issue description, you could put:
> BUG=chromium-os:9414 for relevant bug.
> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> File chrome/browser/chromeos/login/wizard_accessibility_handler.cc (right):
> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> chrome/browser/chromeos/login/wizard_accessibility_handler.cc:123: }
> /tmp/mydebug.txt is debugging code, right? If so, please clean up.

done.

> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> chrome/browser/chromeos/login/wizard_accessibility_handler.cc:137: }
> please clean up the debug code.

done.

> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> File chrome/browser/chromeos/login/wizard_accessibility_helper.cc (right):
> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> chrome/browser/chromeos/login/wizard_accessibility_helper.cc:29: //      new
> views::Accelerator(app::VKEY_Z, false, true, true)
> please remove the TODO since you re-enabled the hotkey.

done.

> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> chrome/browser/chromeos/login/wizard_accessibility_helper.cc:135:
"Accessibility
> turned off",
> Do we need to move the string literals into resources grd?
> 
> Out of curiosity, would i18n be supported?

done.

> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> File chrome/browser/chromeos/login/wizard_controller.cc (right):
> 
>
http://codereview.chromium.org/5839003/diff/1/chrome/browser/chromeos/login/w...
> chrome/browser/chromeos/login/wizard_controller.cc:129: if (accel ==
> accel_enable_accessibility_) {
> nit: accel_enable_accessibility_ -> accel_toggle_accessibility_ since the
hotkey
> is toggling instead of enabling now.

done.

[xiyuan, 2010-12-15 23:07:19]

LGTM

http://codereview.chromium.org/5839003/diff/21001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wizard_accessibility_handler.cc (right):

http://codereview.chromium.org/5839003/diff/21001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_accessibility_handler.cc:113: }
nit: you don't need wrap one-liner in "{}"

[Chaitanya, 2010-12-15 23:19:55]

Thanks!
A small change.. I just realized that two files shouldn't have been in this cl:
wizard_accessibility_handler.cc/h ..  so removed them.

One more question below:

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/new_user_view.cc (right):

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/new_user_view.cc:153:
username_field_->SetAccessibleName(std::wstring(L"Username"));
I'd like to i18n the string literal "Username".
How can I convert std::string to std::wstring? Is there a preferred utility
function I should use?

[xiyuan, 2010-12-16 00:09:20]

I think you need wizard_accessibility_handler.cc/h since ToggleAccessibility is
implemented there.

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/new_user_view.cc (right):

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/new_user_view.cc:153:
username_field_->SetAccessibleName(std::wstring(L"Username"));
On 2010/12/15 23:19:56, Chaitanya wrote:
> I'd like to i18n the string literal "Username".
> How can I convert std::string to std::wstring? Is there a preferred utility
> function I should use?

check out base/utf_string_conversions.h, assuming std::string is UTF8, then you
can use UTF8ToWide,

[oshima, 2010-12-16 00:36:53]

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/new_user_view.cc (right):

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/new_user_view.cc:153:
username_field_->SetAccessibleName(std::wstring(L"Username"));
On 2010/12/15 23:19:56, Chaitanya wrote:
> I'd like to i18n the string literal "Username".
> How can I convert std::string to std::wstring? Is there a preferred utility
> function I should use?

Just wondering, why this is not i18n? For pure ascii string,
you can use ASCIIToWide.

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wizard_accessibility_helper.h (right):

http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_accessibility_helper.h:73: void
SetAccessibilityEnabled(bool);
specify arg name

[Chaitanya, 2010-12-16 01:42:42]

On 2010/12/16 00:09:20, xiyuan wrote:
> I think you need wizard_accessibility_handler.cc/h since ToggleAccessibility
is
> implemented there.

ToggleAccessibility is in wizard_accessibility_helper.cc

> 
>
http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
> File chrome/browser/chromeos/login/new_user_view.cc (right):
> 
>
http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
> chrome/browser/chromeos/login/new_user_view.cc:153:
> username_field_->SetAccessibleName(std::wstring(L"Username"));
> On 2010/12/15 23:19:56, Chaitanya wrote:
> > I'd like to i18n the string literal "Username".
> > How can I convert std::string to std::wstring? Is there a preferred utility
> > function I should use?
> 
> check out base/utf_string_conversions.h, assuming std::string is UTF8, then
you
> can use UTF8ToWide,

Done.

[xiyuan, 2010-12-16 17:10:15]

Okay.

On 2010/12/16 01:42:42, Chaitanya wrote:
> On 2010/12/16 00:09:20, xiyuan wrote:
> > I think you need wizard_accessibility_handler.cc/h since ToggleAccessibility
> is
> > implemented there.
> 
> ToggleAccessibility is in wizard_accessibility_helper.cc
> 
> > 
> >
>
http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
> > File chrome/browser/chromeos/login/new_user_view.cc (right):
> > 
> >
>
http://codereview.chromium.org/5839003/diff/59001/chrome/browser/chromeos/log...
> > chrome/browser/chromeos/login/new_user_view.cc:153:
> > username_field_->SetAccessibleName(std::wstring(L"Username"));
> > On 2010/12/15 23:19:56, Chaitanya wrote:
> > > I'd like to i18n the string literal "Username".
> > > How can I convert std::string to std::wstring? Is there a preferred
utility
> > > function I should use?
> > 
> > check out base/utf_string_conversions.h, assuming std::string is UTF8, then
> you
> > can use UTF8ToWide,
> 
> Done.