A hack to work around wxWidgets failure to capture WX_MENU_OPEN events

See the comments.  It's not pretty, but it works.

Fixes: lp:1829640
* https://bugs.launchpad.net/kicad/+bug/1829640

Fixes: lp:1829307
* https://bugs.launchpad.net/kicad/+bug/1829307

Fixes: lp:1594029
* https://bugs.launchpad.net/kicad/+bug/1594029

common/legacy_gal/eda_draw_frame.cpp

@@ -42,6 +42,7 @@
 #include <view/view.h>
 #include <tool/tool_manager.h>
 #include <tool/tool_dispatcher.h>
+#include <tool/action_menu.h>
 #include <tool/actions.h>
 #include <wx/clipbrd.h>
 #include <worksheet_shape_builder.h>
@@ -79,7 +80,8 @@ static const wxString MaxUndoItemsEntry(wxT( "DevelMaxUndoItems" ) );
 
 BEGIN_EVENT_TABLE( EDA_DRAW_FRAME, KIWAY_PLAYER )
     EVT_CHAR_HOOK( EDA_DRAW_FRAME::OnCharHook )
-
+    EVT_MENU_OPEN( EDA_DRAW_FRAME::OnMenuOpen )
+    EVT_MENU_HIGHLIGHT_ALL( EDA_DRAW_FRAME::OnMenuOpen )
     EVT_MOUSEWHEEL( EDA_DRAW_FRAME::OnMouseEvent )
 END_EVENT_TABLE()
 
@@ -276,7 +278,38 @@ void EDA_DRAW_FRAME::OnActivate( wxActivateEvent& event )
 
 void EDA_DRAW_FRAME::OnMenuOpen( wxMenuEvent& event )
 {
-    // TODO Obsolete!
+    // On wxWidgets 3.0.x Windows, EVT_MENU_OPEN and EVT_MENU_HIGHLIGHT events are not
+    // captured by the ACTON_MENU menus.  While it is fixed in wxWidgets 3.1.x, we still
+    // need a solution for the earlier verions.
+    //
+    // This could be made conditional, but for now I'm going to use the same strategy
+    // everywhere so it gets wider testing.
+    // Note that if the conditional compilation is reactivated, the Connect() lines in
+    // ACTION_MENU::setupEvents() will need to be re-enabled.
+//#if defined( __WINDOWS__ ) && wxCHECK_VERSION( 3, 0, 0 ) && !wxCHECK_VERSION( 3, 1, 0 )
+
+    // As if things weren't bad enough, wxWidgets doesn't pass the menu pointer when the
+    // event is a wxEVT_MENU_HIGHLIGHT, so we store the menu from the EVT_MENU_OPEN call.
+    static ACTION_MENU* currentMenu;
+
+    if( event.GetEventType() == wxEVT_MENU_OPEN )
+    {
+        currentMenu = dynamic_cast<ACTION_MENU*>( event.GetMenu() );
+
+        if( currentMenu )
+            currentMenu->OnMenuEvent( event );
+    }
+    else if( event.GetEventType() == wxEVT_MENU_HIGHLIGHT )
+    {
+        if( currentMenu )
+            currentMenu->OnMenuEvent( event );
+    }
+    else if( event.GetEventType() == wxEVT_MENU_CLOSE )
+    {
+        currentMenu = nullptr;
+    }
+//#endif
+
     event.Skip();
 }
 

common/legacy_wx/eda_draw_frame.cpp

@@ -46,7 +46,6 @@
 #include <math/box2.h>
 #include <lockfile.h>
 #include <trace_helpers.h>
-
 #include <wx/clipbrd.h>
 #include <fctsys.h>
 #include <gr_basic.h>
@@ -55,8 +54,6 @@
 #include <base_screen.h>
 #include <confirm.h>
 #include <draw_frame.h>
-
-
 #include <wx/fontdlg.h>
 #include <wx/snglinst.h>
 #include <view/view.h>
@@ -65,7 +62,7 @@
 #include <tool/tool_manager.h>
 #include <tool/tool_dispatcher.h>
 #include <tool/actions.h>
-
+#include <tool/action_menu.h>
 #include <advanced_config.h>
 #include <menus_helpers.h>
 #include <page_info.h>
@@ -105,6 +102,7 @@ BEGIN_EVENT_TABLE( EDA_DRAW_FRAME, KIWAY_PLAYER )
 
     EVT_MOUSEWHEEL( EDA_DRAW_FRAME::OnMouseEvent )
     EVT_MENU_OPEN( EDA_DRAW_FRAME::OnMenuOpen )
+    EVT_MENU_HIGHLIGHT_ALL( EDA_DRAW_FRAME::OnMenuOpen )
     EVT_ACTIVATE( EDA_DRAW_FRAME::OnActivate )
     EVT_MENU_RANGE( ID_ZOOM_BEGIN, ID_ZOOM_END, EDA_DRAW_FRAME::OnZoom )
 
@@ -306,8 +304,37 @@ void EDA_DRAW_FRAME::OnActivate( wxActivateEvent& event )
 
 void EDA_DRAW_FRAME::OnMenuOpen( wxMenuEvent& event )
 {
-    if( m_canvas )
+    // On wxWidgets 3.0.x Windows, EVT_MENU_OPEN ( and other EVT_MENU_xx) events are not
-        m_canvas->SetCanStartBlock( -1 );
+    // captured by the ACTON_MENU menus.  While it is fixed in wxWidgets 3.1.x, we still
+    // need a solution for the earlier verions.
+    //
+    // This could be made conditional, but for now I'm going to use the same strategy
+    // everywhere so it gets wider testing.
+    // Note that if the conditional compilation is reactivated, the Connect() lines in
+    // ACTION_MENU::setupEvents() will need to be re-enabled.
+//#if defined( __WINDOWS__ ) && wxCHECK_VERSION( 3, 0, 0 ) && !wxCHECK_VERSION( 3, 1, 0 )
+
+    // As if things weren't bad enough, wxWidgets doesn't pass the menu pointer when the
+    // event is a wxEVT_MENU_HIGHLIGHT, so we store the menu from the EVT_MENU_OPEN call.
+    static ACTION_MENU* currentMenu;
+
+    if( event.GetEventType() == wxEVT_MENU_OPEN )
+    {
+        currentMenu = dynamic_cast<ACTION_MENU*>( event.GetMenu() );
+
+        if( currentMenu )
+            currentMenu->OnMenuEvent( event );
+    }
+    else if( event.GetEventType() == wxEVT_MENU_HIGHLIGHT )
+    {
+        if( currentMenu )
+            currentMenu->OnMenuEvent( event );
+    }
+    else if( event.GetEventType() == wxEVT_MENU_CLOSE )
+    {
+        currentMenu = nullptr;
+    }
+//#endif
 
     event.Skip();
 }

common/tool/action_menu.cpp

@@ -87,9 +87,11 @@ void ACTION_MENU::SetIcon( const BITMAP_OPAQUE* aIcon )
 
 void ACTION_MENU::setupEvents()
 {
-    Connect( wxEVT_MENU_OPEN, wxMenuEventHandler( ACTION_MENU::onMenuEvent ), NULL, this );
+// See wxWidgets hack in EDA_DRAW_FRAME::OnMenuOpen().
-    Connect( wxEVT_MENU_HIGHLIGHT, wxMenuEventHandler( ACTION_MENU::onMenuEvent ), NULL, this );
+//    Connect( wxEVT_MENU_OPEN, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), NULL, this );
-    Connect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::onMenuEvent ), NULL, this );
+//    Connect( wxEVT_MENU_HIGHLIGHT, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), NULL, this );
+
+    Connect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), NULL, this );
 }
 
 
@@ -322,7 +324,7 @@ void ACTION_MENU::updateHotKeys()
 }
 
 
-void ACTION_MENU::onMenuEvent( wxMenuEvent& aEvent )
+void ACTION_MENU::OnMenuEvent( wxMenuEvent& aEvent )
 {
     OPT_TOOL_EVENT evt;
     wxString menuText;

include/tool/action_menu.h

@@ -144,6 +144,9 @@ public:
     ///> Menu requires updating before display.
     bool m_Dirty;
 
+    ///> The default menu event handler.
+    void OnMenuEvent( wxMenuEvent& aEvent );
+
 protected:
     ///> Returns an instance of this class. It has to be overridden in inheriting classes.
     virtual ACTION_MENU* create() const;
@@ -190,9 +193,6 @@ protected:
     ///> Initializes handlers for events.
     void setupEvents();
 
-    ///> The default menu event handler.
-    void onMenuEvent( wxMenuEvent& aEvent );
-
     ///> Updates hot key settings for TOOL_ACTIONs in this menu.
     void updateHotKeys();