From be61f84af5ea8c5b3ff52b71f49a181872ffd9d5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 26 Nov 2013 14:50:55 +0000 Subject: [PATCH] Yet another fix after wxMenu::Remove() refactoring. wxMenu::Remove() was still broken in wxMSW after r75250, even with the fix in r75290, as wxMSW code relied on the item still being present in wxMenu::m_items. Delay removing it from there until after DoRemove() call to fix this. See #3424. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@75297 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- src/common/menucmn.cpp | 7 ++++++- src/msw/menu.cpp | 3 --- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/common/menucmn.cpp b/src/common/menucmn.cpp index f9091bd7af..bce29b6c54 100644 --- a/src/common/menucmn.cpp +++ b/src/common/menucmn.cpp @@ -428,11 +428,16 @@ wxMenuItem *wxMenuBase::Remove(wxMenuItem *item) // if we get here, the item is valid or one of Remove() functions is broken wxCHECK_MSG( node, NULL, wxT("removing item not in the menu?") ); + // call DoRemove() before removing the item from the list of items as the + // existing code in port-specific implementation may rely on the item still + // being there (this is the case for at least wxMSW) + wxMenuItem* const item2 = DoRemove(item); + // we detach the item, but we do delete the list node (i.e. don't call // DetachNode() here!) m_items.Erase(node); - return DoRemove(item); + return item2; } wxMenuItem *wxMenuBase::DoRemove(wxMenuItem *item) diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp index 3d3ea7e7b0..2b65c0142b 100644 --- a/src/msw/menu.cpp +++ b/src/msw/menu.cpp @@ -793,9 +793,6 @@ wxMenuItem *wxMenu::DoRemove(wxMenuItem *item) node = node->GetNext(); } - // DoRemove() (unlike Remove) can only be called for an existing 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());