Re: [lmi] wx-2.9.5-rc1 regression with conditional controls enablement

[Vadim Zeitlin]

On Mon, 15 Jul 2013 21:54:13 +0000 Greg Chicares <address@hidden> wrote:

GC> To reproduce:
GC> 
GC>   File | New | Illustration
GC>   click "Riders" tab
GC> 
GC> In the "Honeymoon" groupbox, the "Elected" checkbox is enabled,
GC> as it should be. Check it. This should enable the two suboptions
GC> in its groupbox; however, it does not: they remain grayed out.

 After some intensive debugging (finding the place where the control was
actually disabled took me some time, the MvcController-based design has
many of the nice properties but the ease of debugging is not necessarily
one of them), I think I see the problem: when handling UI updates occurring
while the dialog is shown, the code in MvcController::ConditionallyEnable()
only iterates over the current page children. And this used to include all
controls shown on the page. However since r71886 (done on 2012-06-30, we
seem to be really getting more than a share fair of problems happening on
June 30), the children of wxStaticBoxSizer are created as *children* of
their containing wxStaticBox, not its siblings as before. This behaviour is
better for many reasons but it does mean that the "Riders" page doesn't
have any non-static-box children now, which explains why it never updates
them.

 This analysis does explain this part, too:

GC> With "Elected" checked, press "OK", then "Ctrl-E": now the
GC> suboptions are enabled, as they should be. Unchecking "Elected"
GC> should disable them; it does not.

 Because the code executed on the dialog initialization doesn't iterate on
the page children but (AFAICS) on all controls, so it's not affected.

GC> Similarly, on the "Funds" tab, in the groupbox that is also
GC> named "Funds", the "fee" textcontrol should be enabled iff the
GC> radiobox selection is "Override fund", and changing the radiobox
GC> selection should enable or disable the textcontrol immediately;
GC> but it no longer works that way.

 This is also due to the fact that the control is a child of a static box
and hence not a (direct) child of the page any more.


GC> We've found a few other examples where conditional enablement fails.
GC> With a particular proprietary product, for instance, the "Elected"
GC> checkbox for "Honeymoon" should be disabled; instead, it is enabled,
GC> but attempting to check it fails (on mouse-down, the checkbox turns
GC> gray; on mouse-release, a checkmark appears for a few milliseconds,
GC> then vanishes, and the checkbox turns white). We wouldn't be able to
GC> release that to our end users.

 I am not sure if the problem I found explains this behaviour however. But
it would probably be simplest to fix this problem first and then retest if
the problem still remains.

GC> Whether it works or not in a particular case seems to vary from one
GC> machine to another.

 I also can't explain this at all. The behaviour I see should be perfectly
reproducible. But, again, maybe it would be best to fix the problem we know
about first and then see if any gremlins remain.


 Which brings me back to fixing the problem as this is one thing I hadn't
done yet. One solution would be to simply iterate over the children
recursively. But this could result in even further slowdown (which is a
concern in this code and I can confirm that it's not as fast as I'd like it
to be, as a user) because we'd be iterating over many controls
unnecessarily, i.e. most of the sub-elements of composite controls which
don't have any validators associated with them.

 We could avoid this by using a restricted recursion and only recurse
inside wxStaticBoxes but not the other controls. I think this is the
smallest change that would be necessary to make this code work again so I
plan to do this tomorrow if I don't hear anything different from you but I
would be the first to admit that it's not the most elegant solution.

 It would be nicer to somehow reuse the existing "lineage_" as we could,
AFAICS, put only the windows that do have a Transferor associated with them
into it (although there is some code in TestModelViewConsistency which
would need to be updated then, I think).

 Ideal would be to replace a flat vector by a map of parent pages to the
(interesting) controls inside them, then we could iterate over this instead
of iterating over GetChildren().

 What do you think of all these suggestions and would you by chance see a
better one?

 Thanks in advance,
VZ