Re: [lmi] Assertion 'GetEventHandler == this' failed

[Greg Chicares]

On 2017-02-16 01:59, Vadim Zeitlin wrote:
> On Wed, 15 Feb 2017 21:50:40 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> To reproduce:
> GC>   File | New | Census
> GC>   Census | Edit cell
> GC>   change "Withdrawal" so that it holds an empty string
> GC>   OK
> GC>   Census | Add cell
> GC> Now--in the census manager, without opening the tabbed dialog--click
> GC> "Withdrawal" in each row until the ellipsis button shows. (You don't
> GC> have to click the button--just make sure it shows, in both rows.)
> GC> Then
> GC>   File | Exit | No
> GC> 
> GC> I see a messagebox with the following contents (retyped manually
> GC> because 'wine' doesn't seem to let me copy and paste them):
> GC> 
> GC> Assertion 'GetEventHandler == this' failed (any pushed event handlers 
> must have been removed).
> GC> [file ../src/common/wincmn.cpp, line 476]
[...]
>  Here is the commit which fixes the problem for me:

Committed and pushed. It prevents the problem noted above. However, it
introduces another problem in the same circumstances: hovering over the
ellipsis button causes the ellipsis button to vanish, apparently because
it causes the editor to close. It also introduces a related problem: if
you pop up a tabbed dialog to edit the cell, then click inside the
"Withdrawal" textcontrol, then hover the mouse over the ellipsis button,
the focus changes to the first control on the tab. Using 'skin.xrc', that
first control is a checkbox, which is duly surrounded with a dotted-line
highlight, but, oddly, there is still a blinking vertical-line caret in
the textcontrol.

Why doesn't this condition:
  is_child_of(wxGetTopLevelParent(event.GetWindow()), button_)
block loss of focus to the tooltip? Isn't button_ its parent?

Maybe we should just get rid of the tooltip. This command
  grep Tip *.?pp
shows that, except for the product editor and the modal InputSequenceEntry
dialog, this ellipsis button is the only tooltip in lmi--so, crucially,
it's the only one that can interfere with focus in the tabbed input dialog
or in the wxDataViewCtrl census manager. [...time passes...] Okay, I've
made that change and pushed it.

When I discussed this with Kim, she pointed out that this isn't the first
time we've had problems with this code--I hadn't remembered these earlier
reports:

http://lists.nongnu.org/archive/html/lmi/2016-05/msg00032.html
| Assertion 'GetEventHandler() == this' failed
| (any pushed event handlers must have been removed).
| [file ../src/common/wincmn.cpp, line 476]

[off-list message dated 2015-01-06T18:00Z]
| Assertion 'GetEventHandler() == this' failed
| (any pushed event handlers must have been removed).
| [file ../src/common/wincmn.cpp, line 478]
  
http://lists.nongnu.org/archive/html/lmi/2011-12/msg00003.html
| ../src/common/wincmn.cpp(468): \
| assert "GetEventHandler() == this" failed

Let's hope the source of those problems has now been eradicated forever.
As for the tooltip, we see no need for it at all. In the user manual,
the third line on this page:
  http://www.nongnu.org/lmi/sequence_input.html
says:
| [picture of ellipsis button] You see this button at the right end of each
| field that can vary by duration. Press it to enter a sequence of values.
which is clear enough for any user who doesn't already know or can't
guess what the button does. It's needless, and it's problematic because
by its nature it grabs focus, whereas the MVC implementation absolutely
does not want focus stolen. We're better off without the tooltip, and
removing it seems to resolve the new problem reported above (in 'wine'
at least).

>  As you can see, I used a lambda instead of a helper function in the code
> above. I rather like doing this in C++11 because this allows to keep the
> function definition close to where it's used

Yes, I like this, too. Without this nested-function equivalent, I would
have aliased the window pointers (untested):

+    wxWindow const* p = event.GetWindow();
+    wxWindow const* q = wxGetTopLevelParent(event.GetWindow());
     if
-        (  is_child_of(event.GetWindow(), this)
-        || is_child_of(wxGetTopLevelParent(event.GetWindow()), button_)
+        (  p && p->GetParent() == this
+        || q && q->GetParent() == button_

which doesn't seem clearer.