Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-26 Thread Ana Krivokapic
On 07/25/2013 01:49 PM, Petr Vobornik wrote:
 On 07/24/2013 03:52 PM, Ana Krivokapic wrote:
 On 07/23/2013 06:09 PM, Petr Vobornik wrote:
 On 07/22/2013 04:46 PM, Ana Krivokapic wrote:
 On 07/18/2013 09:47 AM, Petr Vobornik wrote:
 On 07/17/2013 09:18 PM, Ana Krivokapic wrote:
 Hello,

 This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.


 Hello,

 1) IMO  we should not create attribute which is just a negation of 
 another.

 2) We should add set_enabled method to base widget. Existing set_enabled
 methods should use it and maintain widget output consistent with the
 attribute
 (ie. one should not directly set the attr and should use set_enabled
 instead).
 The method should be also callable when content is not yet created.
 get_enabled methods might become unnecessary - one can get the state form
 'enabled' attribute.


 The attached updated patch implements the following changes:

 1) set_enabled method has been added to the base widget class.
 2) get_enabled/is_enabled methods have been removed.
 3) Widget classes that inherit from the base widget class override the
 set_enabled method where necessary.
 4) Using 'enabled: true/false' in the widget definition should now work
 correctly for all types of widgets.



 Thanks.

 1. set_enabled method in input_widget uses `that.input`. Input widget is a
 base class which doesn't set the property and therefore we can't be certain
 that the descendant will set it. Also it may break when you call
 set_enabled(val) before create() .

 We should test for `that.input` presence.

 Same content-created test should be perform on other places:
 widget.js:1017,1147,2006

 2. The changes in option_widget_base break disabling if user doesn't have
 write-rights. (can be reproduced when navigated (by manual change of url) to
 service in self-service.

 Note the differences between read_only, writable and enabled:
 * read_only - reflects metadata
 * writable - reflects ACL
 * enabled - context specific

 read_only and writable don't offer edit interface (label instead of 
 textbox).
 Enabled controls disabled state of textbox. For some widgets the result 
 might
 be the same (radios, checkboxes).

 option_widget_base.set_enabled should be changed. The mixin overwrites the
 original method and therefore doesn't set 'enabled' property.

 3. multiple_choice_section.set_enabled should be renamed. It's related to
 individual choices and not the widget itself. And then new set_enabled 
 should
 be added which would call the old one for each choice.

 4. widget.js:3870 - not sure if it's needed but if so, one should also 
 change
 `action_clicked` method.


 All fixed. Updated patch attached.

 Thanks for the review.


 1. Following code is incorrect (line 2030):

 var input = $('input[name='+that.name+']', that.table);
 if (input) {
 input.prop('disabled', !enabled);
 }

 It will perform document wide search, when that.table is not set, and find all
 inputs with that.name. The test should be:

 if (that.table) {
 $('input[name='+that.name+']', that.table).prop('disabled',
 !enabled);

Fixed.

 }

 2. There are issues with option_widget_base. This widget is really a beast.

 a) There is an existing issue - that.container is not set - which revealed
 itself in this patch. The result is that nested options are not enabled.
 Attaching a fix.

 b) I have doubts about option_widget_base changes on line ~1023. It will
 most-likely work for our cases, but has unwanted behavior. It will set enabled
 to false if read_only or writable is false. So enabled will remain false even
 when writable/read_only changes back to true. It won't probably happen, but it
 might if user would somehow obtain new rights during Web UI lifetime.

 It may be better if original set_enabled method would be renamed and it would
 only reflect the desired state in UI.

 Proposed changes are also in attached diff.

Agreed, changes squashed to the patch.



 I'm thinking about developing a testing page with various widgets where one
 could test how they behave with various setting.


Good idea, I opened a ticket: https://fedorahosted.org/freeipa/ticket/3818

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From d89a8aa747bf4606e5b62b6182ac8b8e9dd0edf6 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Wed, 17 Jul 2013 21:13:42 +0200
Subject: [PATCH] Honor 'enabled' option for widgets.

https://fedorahosted.org/freeipa/ticket/3793
---
 install/ui/src/freeipa/association.js |   1 -
 install/ui/src/freeipa/dns.js |   3 +-
 install/ui/src/freeipa/facet.js   |   2 +-
 install/ui/src/freeipa/rule.js|   2 -
 install/ui/src/freeipa/widget.js  | 142 ++
 5 files changed, 93 insertions(+), 57 deletions(-)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index 

Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-26 Thread Petr Vobornik

On 07/26/2013 01:56 PM, Ana Krivokapic wrote:

On 07/25/2013 01:49 PM, Petr Vobornik wrote:

On 07/24/2013 03:52 PM, Ana Krivokapic wrote:

On 07/23/2013 06:09 PM, Petr Vobornik wrote:

On 07/22/2013 04:46 PM, Ana Krivokapic wrote:

On 07/18/2013 09:47 AM, Petr Vobornik wrote:

On 07/17/2013 09:18 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.



Hello,

1) IMO  we should not create attribute which is just a negation of another.

2) We should add set_enabled method to base widget. Existing set_enabled
methods should use it and maintain widget output consistent with the
attribute
(ie. one should not directly set the attr and should use set_enabled
instead).
The method should be also callable when content is not yet created.
get_enabled methods might become unnecessary - one can get the state form
'enabled' attribute.



The attached updated patch implements the following changes:

1) set_enabled method has been added to the base widget class.
2) get_enabled/is_enabled methods have been removed.
3) Widget classes that inherit from the base widget class override the
set_enabled method where necessary.
4) Using 'enabled: true/false' in the widget definition should now work
correctly for all types of widgets.




Thanks.

1. set_enabled method in input_widget uses `that.input`. Input widget is a
base class which doesn't set the property and therefore we can't be certain
that the descendant will set it. Also it may break when you call
set_enabled(val) before create() .

We should test for `that.input` presence.

Same content-created test should be perform on other places:
widget.js:1017,1147,2006

2. The changes in option_widget_base break disabling if user doesn't have
write-rights. (can be reproduced when navigated (by manual change of url) to
service in self-service.

Note the differences between read_only, writable and enabled:
* read_only - reflects metadata
* writable - reflects ACL
* enabled - context specific

read_only and writable don't offer edit interface (label instead of textbox).
Enabled controls disabled state of textbox. For some widgets the result might
be the same (radios, checkboxes).

option_widget_base.set_enabled should be changed. The mixin overwrites the
original method and therefore doesn't set 'enabled' property.

3. multiple_choice_section.set_enabled should be renamed. It's related to
individual choices and not the widget itself. And then new set_enabled should
be added which would call the old one for each choice.

4. widget.js:3870 - not sure if it's needed but if so, one should also change
`action_clicked` method.



All fixed. Updated patch attached.

Thanks for the review.



1. Following code is incorrect (line 2030):

 var input = $('input[name='+that.name+']', that.table);
 if (input) {
 input.prop('disabled', !enabled);
 }

It will perform document wide search, when that.table is not set, and find all
inputs with that.name. The test should be:

 if (that.table) {
 $('input[name='+that.name+']', that.table).prop('disabled',
!enabled);


Fixed.


 }

2. There are issues with option_widget_base. This widget is really a beast.

a) There is an existing issue - that.container is not set - which revealed
itself in this patch. The result is that nested options are not enabled.
Attaching a fix.

b) I have doubts about option_widget_base changes on line ~1023. It will
most-likely work for our cases, but has unwanted behavior. It will set enabled
to false if read_only or writable is false. So enabled will remain false even
when writable/read_only changes back to true. It won't probably happen, but it
might if user would somehow obtain new rights during Web UI lifetime.

It may be better if original set_enabled method would be renamed and it would
only reflect the desired state in UI.

Proposed changes are also in attached diff.


Agreed, changes squashed to the patch.




I'm thinking about developing a testing page with various widgets where one
could test how they behave with various setting.



Good idea, I opened a ticket: https://fedorahosted.org/freeipa/ticket/3818



ACK and pushed to master.
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-25 Thread Petr Vobornik

On 07/24/2013 03:52 PM, Ana Krivokapic wrote:

On 07/23/2013 06:09 PM, Petr Vobornik wrote:

On 07/22/2013 04:46 PM, Ana Krivokapic wrote:

On 07/18/2013 09:47 AM, Petr Vobornik wrote:

On 07/17/2013 09:18 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.



Hello,

1) IMO  we should not create attribute which is just a negation of another.

2) We should add set_enabled method to base widget. Existing set_enabled
methods should use it and maintain widget output consistent with the attribute
(ie. one should not directly set the attr and should use set_enabled instead).
The method should be also callable when content is not yet created.
get_enabled methods might become unnecessary - one can get the state form
'enabled' attribute.



The attached updated patch implements the following changes:

1) set_enabled method has been added to the base widget class.
2) get_enabled/is_enabled methods have been removed.
3) Widget classes that inherit from the base widget class override the
set_enabled method where necessary.
4) Using 'enabled: true/false' in the widget definition should now work
correctly for all types of widgets.




Thanks.

1. set_enabled method in input_widget uses `that.input`. Input widget is a
base class which doesn't set the property and therefore we can't be certain
that the descendant will set it. Also it may break when you call
set_enabled(val) before create() .

We should test for `that.input` presence.

Same content-created test should be perform on other places:
widget.js:1017,1147,2006

2. The changes in option_widget_base break disabling if user doesn't have
write-rights. (can be reproduced when navigated (by manual change of url) to
service in self-service.

Note the differences between read_only, writable and enabled:
* read_only - reflects metadata
* writable - reflects ACL
* enabled - context specific

read_only and writable don't offer edit interface (label instead of textbox).
Enabled controls disabled state of textbox. For some widgets the result might
be the same (radios, checkboxes).

option_widget_base.set_enabled should be changed. The mixin overwrites the
original method and therefore doesn't set 'enabled' property.

3. multiple_choice_section.set_enabled should be renamed. It's related to
individual choices and not the widget itself. And then new set_enabled should
be added which would call the old one for each choice.

4. widget.js:3870 - not sure if it's needed but if so, one should also change
`action_clicked` method.



All fixed. Updated patch attached.

Thanks for the review.



1. Following code is incorrect (line 2030):

var input = $('input[name='+that.name+']', that.table);
if (input) {
input.prop('disabled', !enabled);
}

It will perform document wide search, when that.table is not set, and 
find all inputs with that.name. The test should be:


if (that.table) {
$('input[name='+that.name+']', 
that.table).prop('disabled', !enabled);

}

2. There are issues with option_widget_base. This widget is really a beast.

a) There is an existing issue - that.container is not set - which 
revealed itself in this patch. The result is that nested options are not 
enabled. Attaching a fix.


b) I have doubts about option_widget_base changes on line ~1023. It will 
most-likely work for our cases, but has unwanted behavior. It will set 
enabled to false if read_only or writable is false. So enabled will 
remain false even when writable/read_only changes back to true. It won't 
probably happen, but it might if user would somehow obtain new rights 
during Web UI lifetime.


It may be better if original set_enabled method would be renamed and it 
would only reflect the desired state in UI.


Proposed changes are also in attached diff.


I'm thinking about developing a testing page with various widgets where 
one could test how they behave with various setting.


--
Petr Vobornik
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 739abcd..f6bcd64 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -910,7 +910,7 @@ IPA.option_widget_base = function(spec, that) {
 
 var parents_selected = [];
 
-$(that._selector+':checked', that.container).each(function() {
+$(that._selector+':checked', that.$node).each(function() {
 var value = $(this).val();
 var option = that.get_option(value);
 if (option  option.nested) {
@@ -924,7 +924,7 @@ IPA.option_widget_base = function(spec, that) {
 
 if (option.nested) {
 var selected = parents_selected.indexOf(option.value)  -1;
-option.widget.set_enabled(selected, true);
+option.widget.update_enabled(selected, true);
 }
 }
 }
@@ -937,7 +937,7 @@ IPA.option_widget_base = 

Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-24 Thread Ana Krivokapic
On 07/23/2013 06:09 PM, Petr Vobornik wrote:
 On 07/22/2013 04:46 PM, Ana Krivokapic wrote:
 On 07/18/2013 09:47 AM, Petr Vobornik wrote:
 On 07/17/2013 09:18 PM, Ana Krivokapic wrote:
 Hello,

 This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.


 Hello,

 1) IMO  we should not create attribute which is just a negation of another.

 2) We should add set_enabled method to base widget. Existing set_enabled
 methods should use it and maintain widget output consistent with the 
 attribute
 (ie. one should not directly set the attr and should use set_enabled 
 instead).
 The method should be also callable when content is not yet created.
 get_enabled methods might become unnecessary - one can get the state form
 'enabled' attribute.


 The attached updated patch implements the following changes:

 1) set_enabled method has been added to the base widget class.
 2) get_enabled/is_enabled methods have been removed.
 3) Widget classes that inherit from the base widget class override the
 set_enabled method where necessary.
 4) Using 'enabled: true/false' in the widget definition should now work
 correctly for all types of widgets.



 Thanks.

 1. set_enabled method in input_widget uses `that.input`. Input widget is a
 base class which doesn't set the property and therefore we can't be certain
 that the descendant will set it. Also it may break when you call
 set_enabled(val) before create() .

 We should test for `that.input` presence.

 Same content-created test should be perform on other places:
 widget.js:1017,1147,2006

 2. The changes in option_widget_base break disabling if user doesn't have
 write-rights. (can be reproduced when navigated (by manual change of url) to
 service in self-service.

 Note the differences between read_only, writable and enabled:
 * read_only - reflects metadata
 * writable - reflects ACL
 * enabled - context specific

 read_only and writable don't offer edit interface (label instead of textbox).
 Enabled controls disabled state of textbox. For some widgets the result might
 be the same (radios, checkboxes).

 option_widget_base.set_enabled should be changed. The mixin overwrites the
 original method and therefore doesn't set 'enabled' property.

 3. multiple_choice_section.set_enabled should be renamed. It's related to
 individual choices and not the widget itself. And then new set_enabled should
 be added which would call the old one for each choice.

 4. widget.js:3870 - not sure if it's needed but if so, one should also change
 `action_clicked` method.


All fixed. Updated patch attached.

Thanks for the review.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From cd2bd3ad6f4596c56042a6e3d8c76596f7b4e6e8 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Wed, 17 Jul 2013 21:13:42 +0200
Subject: [PATCH] Honor 'enabled' option for widgets.

https://fedorahosted.org/freeipa/ticket/3793
---
 install/ui/src/freeipa/association.js |   1 -
 install/ui/src/freeipa/dns.js |   3 +-
 install/ui/src/freeipa/facet.js   |   2 +-
 install/ui/src/freeipa/rule.js|   2 -
 install/ui/src/freeipa/widget.js  | 127 ++
 5 files changed, 85 insertions(+), 50 deletions(-)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index c60c7b8afe9c16ae55e5147574664c60afc43d3e..ad427d66b6b98119b2eb577ae98e4b7c2f1a6932 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -530,7 +530,6 @@ IPA.association_table_widget = function (spec) {
 $('.action-button', that.table).addClass('action-button-disabled');
 that.unselect_all();
 }
-that.enabled = enabled;
 };
 
 that.select_changed = function() {
diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index b4085fea8b792e7f642a10373207916886ff50be..0a0fd3f85b33f51c474f3e6a47cca00ae9ffcfe9 100644
--- a/install/ui/src/freeipa/dns.js
+++ b/install/ui/src/freeipa/dns.js
@@ -603,7 +603,7 @@ IPA.dnszone_adder_dialog = function(spec) {
 var zone = zone_w.save()[0] || '';
 var ns = ns_w.save()[0] || '';
 
-var zone_is_reverse = !zone_w.is_enabled() ||
+var zone_is_reverse = !zone_w.enabled ||
   ends_with(zone, '.in-addr.arpa.') ||
   ends_with(zone, '.ip6.arpa.');
 var relative_ns = true;
@@ -1767,7 +1767,6 @@ IPA.dns.record_type_table_widget = function(spec) {
 $('.action-button', that.table).addClass('action-button-disabled');
 that.unselect_all();
 }
-that.enabled = enabled;
 };
 
 that.select_changed = function() {
diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 37106e22f44b2fb50fc79b8183cc62e9eb35b6e4..b01452dd718b894ecb66d29f70242779ff75cfa4 100644
--- a/install/ui/src/freeipa/facet.js
+++ 

Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-23 Thread Petr Vobornik

On 07/22/2013 04:46 PM, Ana Krivokapic wrote:

On 07/18/2013 09:47 AM, Petr Vobornik wrote:

On 07/17/2013 09:18 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.



Hello,

1) IMO  we should not create attribute which is just a negation of another.

2) We should add set_enabled method to base widget. Existing set_enabled
methods should use it and maintain widget output consistent with the attribute
(ie. one should not directly set the attr and should use set_enabled instead).
The method should be also callable when content is not yet created.
get_enabled methods might become unnecessary - one can get the state form
'enabled' attribute.



The attached updated patch implements the following changes:

1) set_enabled method has been added to the base widget class.
2) get_enabled/is_enabled methods have been removed.
3) Widget classes that inherit from the base widget class override the
set_enabled method where necessary.
4) Using 'enabled: true/false' in the widget definition should now work
correctly for all types of widgets.




Thanks.

1. set_enabled method in input_widget uses `that.input`. Input widget is 
a base class which doesn't set the property and therefore we can't be 
certain that the descendant will set it. Also it may break when you call 
set_enabled(val) before create() .


We should test for `that.input` presence.

Same content-created test should be perform on other places: 
widget.js:1017,1147,2006


2. The changes in option_widget_base break disabling if user doesn't 
have write-rights. (can be reproduced when navigated (by manual change 
of url) to service in self-service.


Note the differences between read_only, writable and enabled:
* read_only - reflects metadata
* writable - reflects ACL
* enabled - context specific

read_only and writable don't offer edit interface (label instead of 
textbox). Enabled controls disabled state of textbox. For some widgets 
the result might be the same (radios, checkboxes).


option_widget_base.set_enabled should be changed. The mixin overwrites 
the original method and therefore doesn't set 'enabled' property.


3. multiple_choice_section.set_enabled should be renamed. It's related 
to individual choices and not the widget itself. And then new 
set_enabled should be added which would call the old one for each choice.


4. widget.js:3870 - not sure if it's needed but if so, one should also 
change `action_clicked` method.


--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0047 Honor 'enabled' option for widgets

2013-07-22 Thread Ana Krivokapic
On 07/18/2013 09:47 AM, Petr Vobornik wrote:
 On 07/17/2013 09:18 PM, Ana Krivokapic wrote:
 Hello,

 This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793.


 Hello,

 1) IMO  we should not create attribute which is just a negation of another.

 2) We should add set_enabled method to base widget. Existing set_enabled
 methods should use it and maintain widget output consistent with the attribute
 (ie. one should not directly set the attr and should use set_enabled instead).
 The method should be also callable when content is not yet created. 
 get_enabled methods might become unnecessary - one can get the state form
 'enabled' attribute.


The attached updated patch implements the following changes:

1) set_enabled method has been added to the base widget class.
2) get_enabled/is_enabled methods have been removed.
3) Widget classes that inherit from the base widget class override the
set_enabled method where necessary.
4) Using 'enabled: true/false' in the widget definition should now work
correctly for all types of widgets.


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 1b2ba09af0b0dc82b583f114c19496bf7a7309f5 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Wed, 17 Jul 2013 21:13:42 +0200
Subject: [PATCH] Honor 'enabled' option for widgets.

https://fedorahosted.org/freeipa/ticket/3793
---
 install/ui/src/freeipa/association.js |  1 -
 install/ui/src/freeipa/dns.js |  3 +-
 install/ui/src/freeipa/facet.js   |  2 +-
 install/ui/src/freeipa/rule.js|  2 -
 install/ui/src/freeipa/widget.js  | 81 +--
 5 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index c60c7b8afe9c16ae55e5147574664c60afc43d3e..ad427d66b6b98119b2eb577ae98e4b7c2f1a6932 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -530,7 +530,6 @@ IPA.association_table_widget = function (spec) {
 $('.action-button', that.table).addClass('action-button-disabled');
 that.unselect_all();
 }
-that.enabled = enabled;
 };
 
 that.select_changed = function() {
diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index b4085fea8b792e7f642a10373207916886ff50be..0a0fd3f85b33f51c474f3e6a47cca00ae9ffcfe9 100644
--- a/install/ui/src/freeipa/dns.js
+++ b/install/ui/src/freeipa/dns.js
@@ -603,7 +603,7 @@ IPA.dnszone_adder_dialog = function(spec) {
 var zone = zone_w.save()[0] || '';
 var ns = ns_w.save()[0] || '';
 
-var zone_is_reverse = !zone_w.is_enabled() ||
+var zone_is_reverse = !zone_w.enabled ||
   ends_with(zone, '.in-addr.arpa.') ||
   ends_with(zone, '.ip6.arpa.');
 var relative_ns = true;
@@ -1767,7 +1767,6 @@ IPA.dns.record_type_table_widget = function(spec) {
 $('.action-button', that.table).addClass('action-button-disabled');
 that.unselect_all();
 }
-that.enabled = enabled;
 };
 
 that.select_changed = function() {
diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 166203a93cd539a36993cfed7816ca9ab616116a..bf876c4e52637c6da057c15621572bc7ff97e90f 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -1939,7 +1939,7 @@ exp.action_button_widget = IPA.action_button_widget = function(spec) {
 };
 
 that.set_enabled = function(enabled) {
-that.enabled = enabled;
+that.widget_set_enabled(enabled);
 
 if (that.button_element) {
 if (enabled) {
diff --git a/install/ui/src/freeipa/rule.js b/install/ui/src/freeipa/rule.js
index f573b56d469ac8c6652f8e49c66fd6fd1259db90..332342bf8f7806da1c77a61d2da8677944657296 100644
--- a/install/ui/src/freeipa/rule.js
+++ b/install/ui/src/freeipa/rule.js
@@ -112,8 +112,6 @@ IPA.rule_association_table_widget = function(spec) {
 
 that.external = spec.external;
 
-that.enabled = spec.enabled !== undefined ? spec.enabled : true;
-
 that.setup_column = function(column, div, record) {
 var suppress_link = false;
 if (that.external) {
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 06fcef563ca416e6e3e1cc454f2e1dd665c68f26..6ea8c339ba6c6f3c78a95f1da33d3b034885ba3e 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -51,6 +51,7 @@ IPA.widget = function(spec) {
 that.measurement_unit = spec.measurement_unit;
 that.entity = IPA.get_entity(spec.entity); //some old widgets still need it
 that.facet = spec.facet;
+that.enabled = spec.enabled === undefined ? true : spec.enabled;
 
 that.create = function(container) {
 container.addClass('widget');
@@ -60,6 +61,10 @@ IPA.widget = function(spec) {
 that.clear =