Autofill button should get enabled and disabled according to the policy value.

Autofill button should get enabled and disabled according to the policy value.

BUG=82912
TEST=Manually set the policy and observe the button gets disabled.


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

[pastarmovj, 2011-07-07 16:51:27]

Please review this small change that fixes a minor UI issue with the AutoFill
button not being disabled when its policy is set.

[stuartmorgan, 2011-07-07 21:07:11]

LGTM

[Joao da Silva, 2011-07-08 08:56:28]

This certainly fixes the problem, but please consider the comments below and let
me know what you think. Thanks!

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/personal_options.js (right):

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:127: }
I'd suggest handling updates to the pref here, similar to PasswordManagement
above. If the control is disabled because we're in Guest mode, don't react to
changes; otherwise listen for updates to the pref from here.

Preferences.getInstance().addEventListener() can be used to track the pref
value, without introducing new browser<->JS calls from the UIHandler.

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:202:
$('autofill-settings').disabled = !enabled;
This can reenable the control when it was disabled because we are in Guest mode
(see PersonalOptions.disableAutofillManagement below on line 241).

[pastarmovj, 2011-07-08 09:33:30]

Following Joao's advice I moved all the handling of the pref change to the JS
code. Nice catch tanks! 

@Joao: Please have a look if I did it like we talked it should be.

@Stuart: Are you also still ok with that?

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/personal_options.js (right):

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:127: }
True good idea. This will save one unneeded IPC call too I guess.

On 2011/07/08 08:56:28, Joao da Silva wrote:
> I'd suggest handling updates to the pref here, similar to PasswordManagement
> above. If the control is disabled because we're in Guest mode, don't react to
> changes; otherwise listen for updates to the pref from here.
> 
> Preferences.getInstance().addEventListener() can be used to track the pref
> value, without introducing new browser<->JS calls from the UIHandler.

http://codereview.chromium.org/7322014/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:202:
$('autofill-settings').disabled = !enabled;
You are right. Good catch!

On 2011/07/08 08:56:28, Joao da Silva wrote:
> This can reenable the control when it was disabled because we are in Guest
mode
> (see PersonalOptions.disableAutofillManagement below on line 241).

[Joao da Silva, 2011-07-08 09:40:52]

LGTM!

[stuartmorgan, 2011-07-08 15:22:36]

Still LGTM (although I'm not particularly familiar with this code)

[commit-bot: I haz the power, 2011-07-08 18:47:02]

Change committed as 91882