From de5ba70203f18fde148c71495ab5ee5aab7540a3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 24 Aug 2018 19:03:15 +0200 Subject: [PATCH] Fix handling events from their items in submenu itself This previously worked in wxGTK, but not in wxMSW and even under wxGTK it could be surprising that the submenu got the event, but its parent menu did not. Make things consistent between the platforms and send the event to the menu directly containing it first, but then also to its parent menu(s). Document the new behaviour and verify that it works as intended with a new unit test. Closes #18202. --- docs/changes.txt | 1 + interface/wx/menu.h | 22 ++++++++++++---------- src/common/menucmn.cpp | 31 ++++++++++++++++--------------- src/msw/menu.cpp | 2 +- tests/events/propagation.cpp | 20 +++++++++++++++++--- 5 files changed, 47 insertions(+), 29 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index c785d23c77..d40be39dc9 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -112,6 +112,7 @@ All (GUI): - Add support for style="page-break-inside:avoid" to
in wxHTML. - Support strike-through in wxDataViewItem attributes (approach, Igor Korot). - Allow distinguishing between user- and script-opened windows in wxWebView. +- Allow binding to events generated by their items in submenus too. wxGTK: diff --git a/interface/wx/menu.h b/interface/wx/menu.h index 3157e3a75b..ace479f0dc 100644 --- a/interface/wx/menu.h +++ b/interface/wx/menu.h @@ -482,18 +482,20 @@ public: @section menu_eventhandling Event handling - If the menu is part of a menubar, then wxMenuBar event processing is used. + Event handlers for the commands generated by the menu items can be + connected directly to the menu object itself using wxEvtHandler::Bind(). If + this menu is a submenu of another one, the events from its items can also + be processed in the parent menu and so on, recursively. - With a popup menu (see wxWindow::PopupMenu), there is a variety of ways to - handle a menu selection event (@c wxEVT_MENU): - - Provide @c EVT_MENU handlers in the window which pops up the menu, or in an - ancestor of that window (the simplest method); - - Derive a new class from wxMenu and define event table entries using the @c EVT_MENU macro; - - Set a new event handler for wxMenu, through wxEvtHandler::SetNextHandler, - specifying an object whose class has @c EVT_MENU entries; + If the menu is part of a menu bar, then events can also be handled in + wxMenuBar object. - Note that instead of static @c EVT_MENU macros you can also use dynamic - connection; see @ref overview_events_bind. + Finally, menu events can also be handled in the associated window, which is + either the wxFrame associated with the menu bar this menu belongs to or the + window for which wxWindow::PopupMenu() was called for the popup menus. + + See @ref overview_events_bind for how to bind event handlers to the various + objects. @library{wxcore} @category{menus} diff --git a/src/common/menucmn.cpp b/src/common/menucmn.cpp index 7c12f30765..1c6fd085df 100644 --- a/src/common/menucmn.cpp +++ b/src/common/menucmn.cpp @@ -648,11 +648,12 @@ bool wxMenuBase::DoProcessEvent(wxMenuBase* menu, wxEvent& event, wxWindow* win) { event.SetEventObject(menu); - if ( menu ) - { - wxMenuBar* const mb = menu->GetMenuBar(); + wxMenuBar* const mb = menu ? menu->GetMenuBar() : NULL; - // Try the menu's event handler first + // Process event in the menu itself and all its parent menus, if it's a + // submenu, first. + for ( ; menu; menu = menu->GetParent() ) + { wxEvtHandler *handler = menu->GetEventHandler(); if ( handler ) { @@ -666,19 +667,19 @@ bool wxMenuBase::DoProcessEvent(wxMenuBase* menu, wxEvent& event, wxWindow* win) if ( handler->SafelyProcessEvent(event) ) return true; } + } - // If this menu is part of the menu bar, try the event there. this - if ( mb ) - { - if ( mb->HandleWindowEvent(event) ) - return true; + // If this menu is part of the menu bar, try the event there. + if ( mb ) + { + if ( mb->HandleWindowEvent(event) ) + return true; - // If this already propagated it upwards to the window containing - // the menu bar, we don't have to handle it in this window again - // below. - if ( event.ShouldPropagate() ) - return false; - } + // If this already propagated it upwards to the window containing + // the menu bar, we don't have to handle it in this window again + // below. + if ( event.ShouldPropagate() ) + return false; } // Try the window the menu was popped up from. diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp index 4ebd6a5d2d..4705b802c9 100644 --- a/src/msw/menu.cpp +++ b/src/msw/menu.cpp @@ -795,7 +795,7 @@ bool wxMenu::MSWCommand(WXUINT WXUNUSED(param), WXWORD id_) } } - SendEvent(id, checked); + item->GetMenu()->SendEvent(id, checked); } return true; diff --git a/tests/events/propagation.cpp b/tests/events/propagation.cpp index 8c58d662fd..c0d302b7ee 100644 --- a/tests/events/propagation.cpp +++ b/tests/events/propagation.cpp @@ -445,11 +445,14 @@ wxMenu* CreateTestMenu(wxFrame* frame) // reliable than using wxUIActionSimulator and currently works in all ports as // they all call wxMenuBase::SendEvent() from their respective menu event // handlers. -#define ASSERT_MENU_EVENT_RESULT(menu, result) \ - g_str.clear(); \ - menu->SendEvent(wxID_APPLY); \ +#define ASSERT_MENU_EVENT_RESULT_FOR(cmd, menu, result) \ + g_str.clear(); \ + menu->SendEvent(cmd); \ CHECK( g_str == result ) +#define ASSERT_MENU_EVENT_RESULT(menu, result) \ + ASSERT_MENU_EVENT_RESULT_FOR(wxID_APPLY, menu, result) + void EventPropagationTestCase::MenuEvent() { wxFrame* const frame = static_cast(wxTheApp->GetTopWindow()); @@ -472,6 +475,17 @@ void EventPropagationTestCase::MenuEvent() ASSERT_MENU_EVENT_RESULT( menu, "aomA" ); + // Check that a handler can also be attached to a submenu. + wxMenu* const submenu = new wxMenu; + submenu->Append(wxID_ABOUT); + menu->Append(wxID_ANY, "Submenu", submenu); + + TestMenuEvtHandler hs('s'); // 's' for "submenu" + submenu->SetNextHandler(&hs); + wxON_BLOCK_EXIT_OBJ1( *submenu, + wxEvtHandler::SetNextHandler, (wxEvtHandler*)NULL ); + ASSERT_MENU_EVENT_RESULT_FOR( wxID_ABOUT, submenu, "aosomA" ); + // Test that the event handler associated with the menu bar gets the event. TestMenuEvtHandler hb('b'); // 'b' for "menu Bar" mb->PushEventHandler(&hb);