Don't pretend static box with enabled label is disabled

Trying to be smart by setting m_isEnabled to false in
wxStaticBox::Enable() without actually disabling the box itself (because
it can't be done if its label window is to remain enabled) didn't really
work. For example, it was impossible to TAB to a checkbox label of the
box when it was disabled, because keyboard navigation (correctly)
doesn't recurse into disabled windows and there could be similar
problems with any other code iterating over windows and skipping over
the disabled ones.

So, finally, simplify things and keep m_isEnabled in sync with the real
box state, even if this, counter-intuitively, means that IsEnabled() on
the box returns true after calling Enable(false) on it.

This also reverts 4ee35cf5ee569b6ee6c7d0d5702484d4d2a20f96 ("Don't
disable wxStaticBox children at wx level when disabling it") as we can't
avoid really disabling the children any more now that their parent is
not disabled: without this, their IsEnabled() would return true, i.e.
they wouldn't be disabled at all, from the program point of view. This
is unfortunate for the reasons that originally motivated that commit,
i.e. if some wxStaticBox child is disabled, disabling and re-enabling
the box will now re-enable this child, even if it shouldn't, but seems
impossible to avoid. The only possible alternative is to modify
IsEnabled() to add some wxStaticBox-specific hook to it, e.g. instead of
calling GetParent()->IsEnabled() there, we could call some now
AreChildrenEnable() method, which would delegate to IsEnabled() by
default but overridden in wxStaticBox. However this seems complicated,
and will add an extra virtual function call to all (frequently
happening) IsEnabled() calls.

include/wx/statbox.h

@@ -54,6 +54,13 @@ protected:
     // static box and will be deleted when it is.
     wxWindow* m_labelWin;
 
+    // For boxes with window label this member variable is used instead of
+    // m_isEnabled to remember the last value passed to Enable(). It is
+    // required because the box itself doesn't get disabled by Enable(false) in
+    // this case (see comments in Enable() implementation), and m_isEnabled
+    // must correspond to its real state.
+    bool m_areChildrenEnabled;
+
     wxDECLARE_NO_COPY_CLASS(wxStaticBoxBase);
 };
 

include/wx/window.h

@@ -1580,11 +1580,6 @@ public:
         return false;
     }
 
-    // Recursively call our own and our children DoEnable() when the
-    // enabled/disabled status changed because a parent window had been
-    // enabled/disabled. This is only used by the library implementation.
-    void NotifyWindowOnEnableChange(bool enabled);
-
 
 protected:
     // helper for the derived class Create() methods: the first overload, with
@@ -1896,6 +1891,11 @@ protected:
     static void NotifyCaptureLost();
 
 private:
+    // recursively call our own and our children DoEnable() when the
+    // enabled/disabled status changed because a parent window had been
+    // enabled/disabled
+    void NotifyWindowOnEnableChange(bool enabled);
+
 #if wxUSE_MENUS
     // temporary event handlers used by GetPopupMenuSelectionFromUser()
     void InternalOnPopupMenu(wxCommandEvent& event);

interface/wx/statbox.h

@@ -182,6 +182,21 @@ public:
                         });
         @endcode
         does work as expected.
+
+        Please note that overriding Enable() to not actually disable this
+        window itself has two possibly unexpected consequences:
+
+        - The box retains its enabled status, i.e. IsEnabled() still returns
+          @true, after calling @c Enable(false).
+        - The box children are enabled or disabled when the box is, which can
+          result in the loss of their original state. E.g. if a box child is
+          initially disabled, then the box itself is disabled and, finally, the
+          box is enabled again, this child will end up being enabled too (this
+          wouldn't happen with any other parent window as its children would
+          inherit the disabled state from the parent instead of being really
+          disabled themselves when it is disabled). To avoid this problem,
+          consider using ::wxEVT_UPDATE_UI to ensure that the child state is
+          always correct or restoring it manually after re-enabling the box.
      */
     virtual bool Enable(bool enable = true);
 };

src/common/statboxcmn.cpp

@@ -32,6 +32,7 @@ extern WXDLLEXPORT_DATA(const char) wxStaticBoxNameStr[] = "groupBox";
 wxStaticBoxBase::wxStaticBoxBase()
 {
     m_labelWin = NULL;
+    m_areChildrenEnabled = true;
 
 #ifndef __WXGTK__
     m_container.DisableSelfFocus();
@@ -88,47 +89,25 @@ bool wxStaticBoxBase::Enable(bool enable)
     // also disabled the label control.
     //
     // Unfortunately it is _not_ enough to just disable the box and then enable
-    // the label window as it would still remain disabled under some platforms
+    // the label window as it would still remain disabled for as long as its
-    // (those where wxHAS_NATIVE_ENABLED_MANAGEMENT is defined, e.g. wxGTK) for
+    // parent is disabled. So we avoid disabling the box at all in this case
-    // as long as its parent is disabled. So we avoid disabling the box at all
+    // and only disable its children.
-    // in this case and only disable its children, but still pretend that the
+    if ( m_labelWin )
-    // box is disabled by updating its m_isEnabled, as it would be surprising
-    // if IsEnabled() didn't return false after disabling the box, for example.
-    //
-    // Finally note that this really needs to be done only when disabling the
-    // box and not when re-enabling it, at least for the platforms without
-    // native enabled state management (e.g. wxMSW) because otherwise we could
-    // have a bug in the following scenario:
-    //
-    //  0. The box is initially enabled
-    //  1. Its parent gets disabled, calling DoEnable(false) on the box and all
-    //     its children from its NotifyWindowOnEnableChange() (including the
-    //     label).
-    //  2. The box is itself disabled (for whatever app logic reason).
-    //  3. The parent gets enabled but this time doesn't do anything with the
-    //     box, because it should continue being disabled.
-    //  4. The box is re-enabled -- but remains actually disabled as
-    //     DoEnable(true) was never called on it (nor on the label).
-    //
-    // To avoid this possibility, we always call the base class version, which
-    // does call DoEnable(true) on the box itself and all its children,
-    // including the label, when re-enabling it.
-    if ( m_labelWin && !enable )
     {
-        if ( enable == IsThisEnabled() )
+        if ( enable == m_areChildrenEnabled )
             return false;
 
+        m_areChildrenEnabled = enable;
+
         const wxWindowList& children = GetChildren();
         for ( wxWindowList::const_iterator i = children.begin();
               i != children.end();
               ++i )
         {
             if ( *i != m_labelWin )
-                (*i)->NotifyWindowOnEnableChange(enable);
+                (*i)->Enable(enable);
         }
 
-        m_isEnabled = enable;
-
         return true;
     }
 #endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX