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

[Vadim Zeitlin]

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]
GC> 
GC> Vadim--Should this be fixed in lmi, or in wx?

 Sorry, my initial conclusion was wrong, this is, after all, a bug in lmi
code and I don't think wxWidgets can be blamed for it because wxDVC can't
do anything if we hide focus events from it, as I unfortunately did in
88f6e9d4aa30b49509fc108dbb269e8718e4cd28 which moved the code from
InputSequenceEditor child controls to the composite control itself, but
forgot to update the references to GetParent() which were now wrong.

 Here is the commit which fixes the problem for me:
---------------------------------- >8 --------------------------------------
commit b3580d6980c966e15624dd94d38d09661b77f83e
Author: Vadim Zeitlin <address@hidden>
Date:   Thu Feb 16 02:53:16 2017 +0100

    Fix check for switching focus inside InputSequenceEntry
    
    The code checking whether focus remains inside the same InputSequenceEntry
    wasn't updated after refactoring of 88f6e9d4aa30b49509fc108dbb269e8718e4cd28
    and could return wrong results, e.g. the check failed when focus switched to
    the subwindow of wxDataViewCtrl which was the parent of InputSequenceEntry
    itself and so the in-place editor wasn't closed in this case, allowing to 
open
    more than one editor at once, which is supposed to be impossible.
    
    Also invert the check direction to make it more natural and use a helper
    is_child_of() function to make the parent checks more readable.

diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index d8e2937..aa705e2 100644
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -1492,15 +1492,24 @@ std::string InputSequenceEntry::field_name() const
 
 void InputSequenceEntry::UponChildKillFocus(wxFocusEvent& event)
 {
-    // Don't notify the parent (and thus wxDataViewCtrl) of focus change if 
its within this
-    // InputSequenceEntry composite control or a InputSequenceEditor window 
opened from it.
-    if(nullptr == event.GetWindow() ||
-        (event.GetWindow()->GetParent() != GetParent() &&
-        wxGetTopLevelParent(event.GetWindow())->GetParent() != this))
+    // Check whether the given possibly null window is a child of another one.
+    auto const is_child_of = [](wxWindow* c, wxWindow* p)
         {
-        GetParent()->ProcessWindowEvent(event);
+        return c && c->GetParent() == p;
+        };
+
+    // Suppress normal focus loss event processing if the focus simply goes to
+    // another element of this composite window or changes inside an
+    // InputSequenceEntry window opened from it and having our button as the
+    // parent: this prevents the in-place editor in the census view from
+    // closing whenever this happens.
+    if(is_child_of(event.GetWindow(), this) ||
+       is_child_of(wxGetTopLevelParent(event.GetWindow()), button_))
+        {
+        event.Skip();
+        return;
         }
-    event.Skip();
+    ProcessWindowEvent(event);
 }
 
 void InputSequenceEntry::UponEnter(wxCommandEvent& event)
@@ -1539,6 +1548,8 @@ void InputSequenceEntry::DoOpenEditor()
 
     // Center the window on the [...] button for best locality -- it will be
     // close to user's point of attention and the mouse cursor.
+    // Note that if the parent used here changes, the code in
+    // UponChildKillFocus() would need to be updated.
     InputSequenceEditor editor(button_, title_, in);
 
     std::string sequence_string = std::string(text_->GetValue());
---------------------------------- >8 --------------------------------------

 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, but if you prefer to not use
non-capturing lambdas (which are equivalent to free functions), it can be
easily replaced by a real function instead, of course.

 Of course, if you have any other questions about this commit, please let
me know, as usual.


GC> We're about to update wx, but if this calls for a wx change, we'll
GC> wait.

 I did find a small bug in wxDVC too and I'll fix it soon, so it would be
nice to get this fix in the next update. Could you please tell me when
exactly would you like to update? I would then test some fixed revision of
wxWidgets with lmi census view more carefully before this is done.

 Thanks in advance,
VZ