Make the accessibility checkbox affect LocalState rather than the...

Make the accessibility checkbox affect LocalState rather than the
user's profile.


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

[csilv, 2011-01-24 18:36:59]

In addition to the items below, in chromeos_system_options.html, remove the
'pref="settings.accessibility"' attribute from the checkbox since we're no
longer using that.

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/system_options_handler.cc (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/system_options_handler.cc:75: printf("***
Initialize\n");
Remove printf.

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/system_options_handler.h (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/system_options_handler.h:1: // Copyright (c) 2010
The Chromium Authors. All rights reserved.
bump copyright to 2011, here and others.

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_system_options.js (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_system_options.js:47:
chrome.send('accessibilityChange', [$('accesibility-check').value]);
Instead of returning the 'value' attribute, i'd prefer we use 'checked'.  So
return this:
  [String($('accessibility-check').checked)]

The handler will get either "true" or "false" which you can use to generate your
boolean.  This is fine:
  bool enabled = (checked_str == "true");

[dmazzoni, 2011-01-24 18:46:10]

Thanks for the review! I made the changes you suggested.

Do you think it'd make sense to support a new attribute like systempref=""? Or
is the number of special cases like this still really small?

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/system_options_handler.cc (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/system_options_handler.cc:75: printf("***
Initialize\n");
On 2011/01/24 18:37:00, csilv wrote:
> Remove printf.

Done.

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/system_options_handler.h (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/system_options_handler.h:1: // Copyright (c) 2010
The Chromium Authors. All rights reserved.
On 2011/01/24 18:37:00, csilv wrote:
> bump copyright to 2011, here and others.

Done.

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_system_options.js (right):

http://codereview.chromium.org/6381007/diff/5001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_system_options.js:47:
chrome.send('accessibilityChange', [$('accesibility-check').value]);
On 2011/01/24 18:37:00, csilv wrote:
> Instead of returning the 'value' attribute, i'd prefer we use 'checked'.  So
> return this:
>   [String($('accessibility-check').checked)]
> 
> The handler will get either "true" or "false" which you can use to generate
your
> boolean.  This is fine:
>   bool enabled = (checked_str == "true");

Done.

[csilv, 2011-01-24 19:02:10]

LGTM after comment change.

http://codereview.chromium.org/6381007/diff/12001/chrome/browser/chromeos/dom...
File chrome/browser/chromeos/dom_ui/system_options_handler.h (right):

http://codereview.chromium.org/6381007/diff/12001/chrome/browser/chromeos/dom...
chrome/browser/chromeos/dom_ui/system_options_handler.h:26: // |args| will
contain the checkbox value as string ("on" or "").
update comment to reflect value of string based on 'checked'.