Subject: faulty logic behind setEnabled being "write" access function for "enabled" property

The conclusion touches one of fundamental properties of all widgets, but bear with me, please. I think the resulting structure, if you accept it, will be much, much more clear. The change I suggest does not change the behaviour of property "enabled" itself, but it allows setting it in a way which makes much more explicit what in fact is being set.

I would like to suggest the following changes:

• add to QWidget a new property "enabledAllowed";
• mark in docs QWidget::setEnabled(bool) as obsolete;
• add to QtVariantPropertyManager possibility of disabling editing based on property's name;
• in QtDesigner: for all widgets disable editing the property "enabled"; "enabledAllowed" should be edited instead;
• in QtDesigner: when loading old .ui file which has "enabled" explicitly set, set "enabledAllowed" instead.

Problem:

• QWidget has property "enabled";
• its value is returned by "enabled()" (returning !WA_Disabled);
• this property has write function "setEnabled";
• BUT: setEnabled does not in fact set value of "enabled" directly (it operates directly on a different attribute: WA_ForceDisabled);
• and the other way around: reading value of "enabled()" does not tell you what was the argument xxx in your call setEnabled(xxx).

I think it is bad if a property access function does not set this property, and I will give you a few closely related examples of what can go wrong. You can handle this behaviour from manually written code, but it will often go wrong for tools (such as QtDesigner) which try to access/edit properties automatically.

Example 1: in Qt Designer

• set "enabled" to "true" for a child; this results in a call setEnabled(true) which:
  • sets WA_ForceDisabled to not set and calls helper function which sets enabled() to true
• now set parent's "enabled" to "false": this results in call setEnabled(false) which:
  • sets WA_ForceDisabled to true, and sets both parent's and child's enabled() to "false"

You would expect that QtDesigner should generate .ui which would result in the same behaviour in your application (i.e. the the same calls in .h resulting from uic: setEnabled(true) for child and setEnabled(false) for parent). But what does QtDesigner do? Well, it does the same as it does for other properties:
1. it says: "oh 'enabled' is set for both parent and child? great, let's add for each of them a section to .ui:"

<property name="enabled">
 <bool>VALUE</bool>
</property>

2. where does "VALUE" come from? well we are talking about property "enabled", right? it's value should be returned by the read access function, "enabled()"; at this moment both parent and child would have enabled() == false, so designer will insert for both:

<property name="enabled">
 <bool>false</bool>
</property>

3. now uic runs: it sees the property, so it adds a call to this property's "write" access function. The result will be a call "setEnabled(false)" for both parent and the child...

In other words, if you have a child widget of a disabled parent widget, you can from the code call both setEnabled(true) and setEnabled(false), and this will simply set WA_ForceDisabled accordingly; but if you have a tool such as QtDesigner+uic which generate for a property "write" calls based on value returned by "read" access function, then for a child of a disabled widget you can only get no calls or "setEnabled(false)", there is no way to cause in the code generated from QtDesigner a call "setEnabled(true)". This

• is limiting;
• leads to buggy behaviour like what I describe below (e.g. you can have in QtDesigner checked checkbox representing a property but saved value will be "false");
• leads to loosing information in the .ui form if you decide to temporarily disable a parent widget (see QTBUG-11267).

Example 2: The second example is a bit more abstract but it shows what I mean by a "more clear structure". Imagine some sort of (e.g. QtDeclarative) binding between properties:

• imagine you bind "enabled" of widget A to follow a bool property p1 of object o1;
• and you bind boolean property p2 of another object o2 to follow "enabled" of A.

Now, let's start with all 3 properties "false", and let's imagine that widget A has a disabled parent. Now let's set p1 to "true":

• setting p1 to true will result a in call to the "write" access function of bound property of A, i.e. "setEnabled(true)";
• but ... A will still have enabled() == false;
• and o2 will not get any calls to setP2.

While there are reasons why "enabled" behaves this way, from the point of view of property binding this is a weird behaviour. (See bellow to see how clear the new structure is.)

Example 3: again in Qt Designer (see QTBUG-11154 and QTBUG-11267):

• you have a disabled groupbox with a child pushbutton;
• you select the child, you see that it has enabled() == false – a checkbox representing this property is unchecked;
• now you can double-click on this checkbox;
• this changes the checkbox to "checked";
• and this generates a call to the "write" function of property "enabled", i.e. setEnabled(true);
• but note that button still has enabled() == false (because setEnabled did NOT set this property, it only cleared the WA_ForceDisabled attribute, and the parent is still disabled);
• now we save this form in Qt Designer;
• Qt Designer will go over all properties which were set explicitly/manually and it will find among them "enabled" of our button;
• so it will save

     <property name="enabled">
      <bool>VALUE</bool>
     </property>
  

• VALUE will come from enabled() of the button, i.e. will be false
• the result is that you see in the interface of Qt Designer a checked checkbox, but the saved property value will be false!
• additionally, if you now select e.g. the main dialog and then select the button again, the property will be re-read again, and the checkbox representing property "enabled" will be unchecked now: you changed nothing, you just selected the same button again and the checkbox changes from checked to unchecked => what is going on is not only counter-intuitive but simply hard to understand at all.

I think it is very sensible how "enabled" behaves (how it propagates from parents to children etc.). The problem is that "setEnabled()" does not in all cases set this property (setEnabled() manipulates WA_ForceDisabled, while enabled() returns a value based on WA_Disabled). Therefore it will be always misleading if any automatic tool (e.g. QtDesigner) treated "setEnabled()" as a "write" access function for property "enabled". (Note also that it is also not OK in such a tool to try to write a value and then test if the property was indeed set: the problem is that current setEnabled() doesn't simply fail to set "enabled" - it additionally manipulates another attribute, WA_ForceDisabled). In a sense declaring setEnabled() as a write access function for "enabled" promises to set "enabled" to true or false; but in fact it does something else instead!

A more correct structure would be:

• property "enabled" should not have a direct "write" access function;
• there should be another property e.g. "forceDisabled", with both read and write access functions and which can be independently set for each widget.

The result would be that "enabled" would still be propagated between parent and child widgets, but only the other property would be set directly by the "write" function and its value would always represent the last write call; and more importantly, based on the value of this property tools which handle properties automatically would know what to do with 100% certainty.

This seems to be a much, much more clear structure than the current structure.

Also, it solves all three example problems mentioned above:

• for Qt Designer => user would be directly editing "forceDisabled", and whatever he/she sets would be stored;
• and for binding => you bind property p1 to "forceDisabled" of widget A and it will always follow; you can bind p2 to "forceDisabled" and it will also follow, or you can bind it to "enabled" but then you know that you bound to a different property!

Two remarks:

• the above new property "forceDisabled" would correspond directly to the underlying widget attribute WA_ForceDisabled. But I also think that there is a point in keeping properties "positive" (thus "checked", "enabled" and not "unchecked" or "disabled"). It's to some extend a matter of aesthetics, but I want to suggest a property "enabledAllowed" (which would be an opposite of "forceDisabled").

• I said that ideally property "enabled" should not have a direct "write" access function. But for now, so that we did not break current applications, "setEnabled()" has to be there as a write access method for property "enabled". Also, so that QtDesigner and uic were able to read and interpret .ui files which set property "enabled".

Corresponding merge request has three parts:

PART 1 of the patch:

• it adds to QWidget a property "enabledAllowed" and read/write access functions;
• keeps current behaviour or enabled() and setEnabled;

  --- a/src/gui/kernel/qwidget.h
  +++ b/src/gui/kernel/qwidget.h
  +    Q_PROPERTY(bool enabledAllowed READ isEnabledAllowed WRITE setEnabledAllowed)
  
  --- a/src/gui/kernel/qwidget.cpp
  +++ b/src/gui/kernel/qwidget.cpp
  @@ -3181,6 +3181,11
   */
   void QWidget::setEnabled(bool enable)
   {
  +    setEnabledAllowed(enable);
  +}
  +
  +void QWidget::setEnabledAllowed(bool enable)
  +{
       Q_D(QWidget);
       setAttribute(Qt::WA_ForceDisabled, !enable);
       d->setEnabled_helper(enable);
  

  and:

  +inline bool QWidget::isEnabledAllowed() const
  +{ return !testAttribute(Qt::WA_ForceDisabled); }
  

  • so setEnabledAllowed and isEnabledAllowed are based on the same widget attribute. I also update the doxygen docs and mark in docs setEnabled(bool) as obsolete.

PART 2 of the patch:

• in designer I want to explicitly disable the misleading possibility of editing "enabled"; "enabledAllowed" should be edited by users instead;
• BUT: property editor is invoked by QtVariantEditorFactory which in principle can be used for not-widgets; for such not-widget objects if they have property "enabled" it would have a different meaning and therefore it would be wrong to simply disable editing of "enabled" always.
• thus I've introduced

  +void QtVariantPropertyManager::addNotEditablePropertyName(const QString &propertyName)
  +bool QtVariantPropertyManager::isNotEditablePropertyName(const QString &propertyName)
  +void QtVariantPropertyManager::resetNotEditablePropertyNames()
  

• in PropertyEditor of QtDesigner I set explicitly:

  +    variantManager->addNotEditablePropertyName("enabled");
  

  This means that for any other tools which would use QtVariantPropertyManager nothing will change.

Note: this change is quite simple, but maybe you have an even easier way to disable editing of "enabled" in QtDesigner?

PART3 of the patch:

Old (current) forms have property "enabled" set explicitly, and it corresponds to call to setEnabled which sets WA_ForceDisabled attribute. In the new situation this can be done more clearly by setting property "enabledAllowed" to the same value instead. This is done by the last part of the patch, which changes the code loading a .ui file into QtDesigner:

--- a/tools/designer/src/components/formeditor/qdesigner_resource.cpp
+++ b/tools/designer/src/components/formeditor/qdesigner_resource.cpp
-        const int index = sheet->indexOf(propertyName);
+        int tempIdx = -1;
+        if ( propertyName == QLatin1String("enabled") ) {
+            // support for old .ui files which tried to set property "enabled" instead of "enabledAllowed":
+            tempIdx = sheet->indexOf("enabledAllowed");
+        } else {
+            tempIdx = sheet->indexOf(propertyName);
+        }
+        const int index = tempIdx;

Thus, all loaded forms focus immediately on editing "enabledAllowed", and not "enabled". In my opinion this is very convenient and much more clear than e.g. waiting for the user to do some first change etc. And again: the main benefit is that for a disabled parent a user can now in QtDesigner set child's enabledAllowed to either true or false, and this setting is respected, preserved and applied as expected (this is not true for current manipulation of "enabled").

This last change changes what is visible in interface of QtDesigner, but also means that if you just open and save .ui file without any changes, and your old file had property enabled == false the new file will have enabledAllowed == false (which, again, does not change the behaviour of resulting application, but makes it much more clear that this is WA_ForceDisabled what is set/manipulated).

Final comments:

• the change in the QWidget's code only expands possibilities by introduction of a new property;

• for designer: as you can see in QTBUG-11154 and QTBUG-11267, QtDesigner does not handle "enabled" in a consistent way; the switch to "enabledAllowed" would be a major benefit there;

• the change in .ui file only represents in a much more clear way what in fact uic currently does: when it encounters "enabled" it calls setEnabled() which in fact does not set directly "enabled" (i.e. WA_Disabled), it sets directly the attribute WA_ForceDisabled! The fact that new .ui files have property "enabledAllowed" set instead of "enabled" makes this behaviour much more explicit; so the change in fact
  • only makes current behaviour much more explicit, there is no change in behaviour of user's applications
  • it enables editing this behaviour in a consistent way (which as you can see in QTBUG-11154 and QTBUG-11267 is right now simply not possible)