Fix menu event handlers calling order.

Pass the menu event to the window associated with the menu first, before
falling back on wxApp.

This required adding yet another flag to keep state in wxEvent but it seems to
be unavoidable as wxMenuBase::SendEvent() calls ProcessEvent() twice and we
must have some way to distinguish the first call from the second one.

Added a test case verifying that the menu events are indeed processed in the
expected order.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@73921 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/event.h

@@ -1000,6 +1000,24 @@ public:
         return false;
     }
 
+    // This is for internal use only and is used for setting, testing and
+    // resetting of m_willBeProcessedAgain flag.
+    void SetWillBeProcessedAgain()
+    {
+        m_willBeProcessedAgain = true;
+    }
+
+    bool WillBeProcessedAgain()
+    {
+        if ( m_willBeProcessedAgain )
+        {
+            m_willBeProcessedAgain = false;
+            return true;
+        }
+
+        return false;
+    }
+
     // This is also used only internally by ProcessEvent() to check if it
     // should process the event normally or only restrict the search for the
     // event handler to this object itself.
@@ -1047,6 +1065,11 @@ protected:
     // once for this event
     bool m_wasProcessed;
 
+    // This one is initially false too, but can be set to true to indicate that
+    // the event will be passed to another handler if it's not processed in
+    // this one.
+    bool m_willBeProcessedAgain;
+
 protected:
     wxEvent(const wxEvent&);            // for implementing Clone()
     wxEvent& operator=(const wxEvent&); // for derived classes operator=()

src/common/event.cpp

@@ -370,6 +370,7 @@ wxEvent::wxEvent(int theId, wxEventType commandType)
     m_isCommandEvent = false;
     m_propagationLevel = wxEVENT_PROPAGATE_NONE;
     m_wasProcessed = false;
+    m_willBeProcessedAgain = false;
 }
 
 wxEvent::wxEvent(const wxEvent& src)
@@ -384,6 +385,7 @@ wxEvent::wxEvent(const wxEvent& src)
     , m_skipped(src.m_skipped)
     , m_isCommandEvent(src.m_isCommandEvent)
     , m_wasProcessed(false)
+    , m_willBeProcessedAgain(false)
 {
 }
 
@@ -403,6 +405,10 @@ wxEvent& wxEvent::operator=(const wxEvent& src)
 
     // don't change m_wasProcessed
 
+    // While the original again could be passed to another handler, this one
+    // isn't going to be processed anywhere else by default.
+    m_willBeProcessedAgain = false;
+
     return *this;
 }
 
@@ -1428,6 +1434,12 @@ bool wxEvtHandler::TryAfter(wxEvent& event)
     if ( GetNextHandler() )
         return GetNextHandler()->TryAfter(event);
 
+    // If this event is going to be processed in another handler next, don't
+    // pass it to wxTheApp now, it will be done from TryAfter() of this other
+    // handler.
+    if ( event.WillBeProcessedAgain() )
+        return false;
+
 #if WXWIN_COMPATIBILITY_2_8
     // as above, call the old virtual function for compatibility
     return TryParent(event);

src/common/menucmn.cpp

@@ -643,22 +643,29 @@ bool wxMenuBase::SendEvent(int itemid, int checked)
     event.SetEventObject(this);
     event.SetInt(checked);
 
-    bool processed = false;
+    wxWindow * const win = GetWindow();
 
     // Try the menu's event handler first
     wxEvtHandler *handler = GetEventHandler();
     if ( handler )
-        processed = handler->SafelyProcessEvent(event);
-
-    // Try the window the menu was popped up from or its menu bar belongs to
-    if ( !processed )
     {
-        wxWindow * const win = GetWindow();
+        // Indicate to the event processing code that we're going to pass this
+        // event to another handler if it's not processed here to prevent it
+        // from passing the event to wxTheApp: this will be done below if we do
+        // have the associated window.
         if ( win )
-            processed = win->HandleWindowEvent(event);
+            event.SetWillBeProcessedAgain();
+
+        if ( handler->SafelyProcessEvent(event) )
+            return true;
     }
 
-    return processed;
+    // Try the window the menu was popped up from or its menu bar belongs to
+    if ( win && win->HandleWindowEvent(event) )
+        return true;
+
+    // Not processed.
+    return false;
 }
 
 // ----------------------------------------------------------------------------

tests/events/propagation.cpp

@@ -24,6 +24,9 @@
     #include "wx/window.h"
 #endif // WX_PRECOMP
 
+#include "wx/frame.h"
+#include "wx/menu.h"
+
 #include "wx/scopeguard.h"
 
 namespace
@@ -81,6 +84,14 @@ struct TestEvtHandler : TestEvtHandlerBase<wxCommandEvent>
     }
 };
 
+struct TestMenuEvtHandler : TestEvtHandlerBase<wxCommandEvent>
+{
+    TestMenuEvtHandler(char tag)
+        : TestEvtHandlerBase<wxCommandEvent>(wxEVT_MENU, tag)
+    {
+    }
+};
+
 struct TestPaintEvtHandler : TestEvtHandlerBase<wxPaintEvent>
 {
     TestPaintEvtHandler(char tag)
@@ -144,7 +155,8 @@ private:
 
 int DoFilterEvent(wxEvent& event)
 {
-    if ( event.GetEventType() == TEST_EVT )
+    if ( event.GetEventType() == TEST_EVT ||
+            event.GetEventType() == wxEVT_MENU )
         g_str += 'a';
 
     return -1;
@@ -152,7 +164,8 @@ int DoFilterEvent(wxEvent& event)
 
 bool DoProcessEvent(wxEvent& event)
 {
-    if ( event.GetEventType() == TEST_EVT )
+    if ( event.GetEventType() == TEST_EVT ||
+            event.GetEventType() == wxEVT_MENU )
         g_str += 'A';
 
     return false;
@@ -181,6 +194,7 @@ private:
         CPPUNIT_TEST( ForwardEvent );
         CPPUNIT_TEST( ScrollWindowWithoutHandler );
         CPPUNIT_TEST( ScrollWindowWithHandler );
+        CPPUNIT_TEST( MenuEvent );
     CPPUNIT_TEST_SUITE_END();
 
     void OneHandler();
@@ -190,6 +204,7 @@ private:
     void ForwardEvent();
     void ScrollWindowWithoutHandler();
     void ScrollWindowWithHandler();
+    void MenuEvent();
 
     DECLARE_NO_COPY_CLASS(EventPropagationTestCase)
 };
@@ -347,3 +362,48 @@ void EventPropagationTestCase::ScrollWindowWithHandler()
     CPPUNIT_ASSERT_EQUAL( "apA", g_str );
 }
 
+// Helper for checking that the menu event processing resulted in the expected
+// output from the handlers.
+void CheckMenuEvent(wxMenu* menu, const char* expected)
+{
+    g_str.clear();
+
+    // Trigger the menu event: this is more reliable than using
+    // wxUIActionSimulator and currently works in all ports as they all call
+    // wxMenuBase::SendEvent() from their respective menu event handlers.
+    menu->SendEvent(wxID_NEW);
+
+    CPPUNIT_ASSERT_EQUAL( expected, g_str );
+}
+
+void EventPropagationTestCase::MenuEvent()
+{
+    // Create a minimal menu bar.
+    wxMenu* const menu = new wxMenu;
+    menu->Append(wxID_NEW);
+    wxMenuBar* const mb = new wxMenuBar;
+    mb->Append(menu, "&Menu");
+
+    wxFrame* const frame = static_cast<wxFrame*>(wxTheApp->GetTopWindow());
+    frame->SetMenuBar(mb);
+    wxON_BLOCK_EXIT_OBJ1( *frame, wxFrame::SetMenuBar, (wxMenuBar*)NULL );
+
+    // Check that wxApp gets the event exactly once.
+    CheckMenuEvent( menu, "aA" );
+
+
+    // Check that the menu event handler is called.
+    TestMenuEvtHandler hm('m'); // 'm' for "menu"
+    menu->SetNextHandler(&hm);
+    wxON_BLOCK_EXIT_OBJ1( *menu,
+                          wxEvtHandler::SetNextHandler, (wxEvtHandler*)NULL );
+    CheckMenuEvent( menu, "aomA" );
+
+
+    // Also test that the window to which the menu belongs gets the event.
+    TestMenuEvtHandler hw('w'); // 'w' for "Window"
+    frame->PushEventHandler(&hw);
+    wxON_BLOCK_EXIT_OBJ1( *frame, wxWindow::PopEventHandler, false );
+
+    CheckMenuEvent( menu, "aomowA" );
+}