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

diff --git a/include/wx/event.h b/include/wx/event.h
index 0aaa2414effda6a35efdba4dc4dfe582d2037a6c..87a1a6a304f07bfccfab7b9fa9c11ec76d60c032 100644 (file)
--- a/include/wx/event.h
+++ b/include/wx/event.h
@@ -1000,6 +1000,24 @@ public:
         return false;
     }
 
         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.
     // 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;
 
     // 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=()
 protected:
     wxEvent(const wxEvent&);            // for implementing Clone()
     wxEvent& operator=(const wxEvent&); // for derived classes operator=()

diff --git a/src/common/event.cpp b/src/common/event.cpp
index c35ba60eb87f81a36f49d8e96823552bcf3e6e27..c7d1d33bde6753459de6f61194dfbf8c0db89dc0 100644 (file)
--- a/src/common/event.cpp
+++ b/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_isCommandEvent = false;
     m_propagationLevel = wxEVENT_PROPAGATE_NONE;
     m_wasProcessed = false;

+    m_willBeProcessedAgain = false;

 }
 
 wxEvent::wxEvent(const wxEvent& src)
 }
 
 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_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
 
 
     // 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;
 }
 
     return *this;
 }
 


@@ -1428,6 +1434,12 @@ bool wxEvtHandler::TryAfter(wxEvent& event)
     if ( GetNextHandler() )
         return GetNextHandler()->TryAfter(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);
 #if WXWIN_COMPATIBILITY_2_8
     // as above, call the old virtual function for compatibility
     return TryParent(event);

diff --git a/src/common/menucmn.cpp b/src/common/menucmn.cpp
index 53745cb0245739d2da525656b76c0a9f591e50cd..55daa81c38e5c041e688f56f640f9abb2e52d5fa 100644 (file)
--- a/src/common/menucmn.cpp
+++ b/src/common/menucmn.cpp
@@ -643,22 +643,29 @@ bool wxMenuBase::SendEvent(int itemid, int checked)
     event.SetEventObject(this);
     event.SetInt(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 )
 
     // 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 )
         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;

 }
 
 // ----------------------------------------------------------------------------
 }
 
 // ----------------------------------------------------------------------------

diff --git a/tests/events/propagation.cpp b/tests/events/propagation.cpp
index a9bfcfba56da0be9c635fa89b6820e6f3c36f8b7..d0d2230ddaa38f2961e0f7ebbc0be9ca147a0b49 100644 (file)
--- a/tests/events/propagation.cpp
+++ b/tests/events/propagation.cpp
@@ -24,6 +24,9 @@
     #include "wx/window.h"
 #endif // WX_PRECOMP
 
     #include "wx/window.h"
 #endif // WX_PRECOMP
 

+#include "wx/frame.h"
+#include "wx/menu.h"
+

 #include "wx/scopeguard.h"
 
 namespace
 #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)
 struct TestPaintEvtHandler : TestEvtHandlerBase<wxPaintEvent>
 {
     TestPaintEvtHandler(char tag)


@@ -144,7 +155,8 @@ private:
 
 int DoFilterEvent(wxEvent& event)
 {
 
 int DoFilterEvent(wxEvent& event)
 {

-    if ( event.GetEventType() == TEST_EVT )
+    if ( event.GetEventType() == TEST_EVT ||
+            event.GetEventType() == wxEVT_MENU )

         g_str += 'a';
 
     return -1;
         g_str += 'a';
 
     return -1;


@@ -152,7 +164,8 @@ int DoFilterEvent(wxEvent& event)
 
 bool DoProcessEvent(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;
         g_str += 'A';
 
     return false;


@@ -181,6 +194,7 @@ private:
         CPPUNIT_TEST( ForwardEvent );
         CPPUNIT_TEST( ScrollWindowWithoutHandler );
         CPPUNIT_TEST( ScrollWindowWithHandler );
         CPPUNIT_TEST( ForwardEvent );
         CPPUNIT_TEST( ScrollWindowWithoutHandler );
         CPPUNIT_TEST( ScrollWindowWithHandler );

+        CPPUNIT_TEST( MenuEvent );

     CPPUNIT_TEST_SUITE_END();
 
     void OneHandler();
     CPPUNIT_TEST_SUITE_END();
 
     void OneHandler();


@@ -190,6 +204,7 @@ private:
     void ForwardEvent();
     void ScrollWindowWithoutHandler();
     void ScrollWindowWithHandler();
     void ForwardEvent();
     void ScrollWindowWithoutHandler();
     void ScrollWindowWithHandler();

+    void MenuEvent();

 
     DECLARE_NO_COPY_CLASS(EventPropagationTestCase)
 };
 
     DECLARE_NO_COPY_CLASS(EventPropagationTestCase)
 };


@@ -347,3 +362,48 @@ void EventPropagationTestCase::ScrollWindowWithHandler()
     CPPUNIT_ASSERT_EQUAL( "apA", g_str );
 }
 
     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" );
+}