From 60542745f6114d4789f9ae355a72d6b62b44513e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= <vaclav@slavik.io>
Date: Mon, 5 Dec 2016 17:59:03 +0100
Subject: [PATCH] Fix accel handling when removing item from submenu

wxMSW propagates accelerators to the top menu in wxMenu::UpdateAccel(),
but the reverse operation in wxMenu::DoRemove() didn't do it, resulting
in leaked leftover accelerator entries that could prevent the same
accelerator from working if an item using it was later added. Fix by
adding RemoveAccel() helper method that behaves analogously to
UpdateAccel().
---
 include/wx/msw/menu.h |  1 +
 src/msw/menu.cpp      | 41 ++++++++++++++++++++++++++++-------------
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/include/wx/msw/menu.h b/include/wx/msw/menu.h
index 7dad184e74..7a85783060 100644
--- a/include/wx/msw/menu.h
+++ b/include/wx/msw/menu.h
@@ -76,6 +76,7 @@ public:
 
     // called by wxMenuItem when its accels changes
     void UpdateAccel(wxMenuItem *item);
+    void RemoveAccel(wxMenuItem *item);
 
     // helper used by wxMenu itself (returns the index in m_accels)
     int FindAccel(int id) const;
diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp
index 95d9585722..6022b8f679 100644
--- a/src/msw/menu.cpp
+++ b/src/msw/menu.cpp
@@ -417,6 +417,33 @@ void wxMenu::UpdateAccel(wxMenuItem *item)
     //else: it is a separator, they can't have accels, nothing to do
 }
 
+void wxMenu::RemoveAccel(wxMenuItem *item)
+{
+    // recurse upwards: we should only modify m_accels of the top level
+    // menus, not of the submenus as wxMenuBar doesn't look at them
+    // (alternative and arguable cleaner solution would be to recurse
+    // downwards in GetAccelCount() and CopyAccels())
+    if ( GetParent() )
+    {
+        GetParent()->RemoveAccel(item);
+        return;
+    }
+
+    // remove the corresponding accel from the accel table
+    int n = FindAccel(item->GetId());
+    if ( n != wxNOT_FOUND )
+    {
+        delete m_accels[n];
+
+        m_accels.RemoveAt(n);
+
+#if wxUSE_OWNER_DRAWN
+        ResetMaxAccelWidth();
+#endif
+    }
+    //else: this item doesn't have an accel, nothing to do
+}
+
 #endif // wxUSE_ACCEL
 
 namespace
@@ -735,19 +762,7 @@ wxMenuItem *wxMenu::DoRemove(wxMenuItem *item)
     }
 
 #if wxUSE_ACCEL
-    // remove the corresponding accel from the accel table
-    int n = FindAccel(item->GetId());
-    if ( n != wxNOT_FOUND )
-    {
-        delete m_accels[n];
-
-        m_accels.RemoveAt(n);
-
-#if wxUSE_OWNER_DRAWN
-        ResetMaxAccelWidth();
-#endif
-    }
-    //else: this item doesn't have an accel, nothing to do
+    RemoveAccel(item);
 #endif // wxUSE_ACCEL
 
     // Update indices of radio groups.