From 90b124835e7bc9328bf397105a8fb3ca16ea3cd1 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 18:03:04 +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().

(backport of 60542745f6114d4789f9ae355a72d6b62b44513e from master)
---
 include/wx/msw/menu.h |  3 +++
 src/msw/menu.cpp      | 41 ++++++++++++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/include/wx/msw/menu.h b/include/wx/msw/menu.h
index 616b3044ad..8ba1094717 100644
--- a/include/wx/msw/menu.h
+++ b/include/wx/msw/menu.h
@@ -94,6 +94,9 @@ public:
 
     // called by wxMenuItem when its accels changes
     void UpdateAccel(wxMenuItem *item);
+#if wxABI_VERSION >= 30002
+    void RemoveAccel(wxMenuItem *item);
+#endif
 
     // 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 0f610c3965..435fcaab9b 100644
--- a/src/msw/menu.cpp
+++ b/src/msw/menu.cpp
@@ -458,6 +458,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
@@ -863,19 +890,7 @@ wxMenuItem *wxMenu::DoRemove(wxMenuItem *item)
     wxCHECK_MSG( node, NULL, wxT("bug in wxMenu::Remove logic") );
 
 #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.