[ wxwindows-Bugs-1538643 ] wxMSW & wxMac: "bug in wxMenu::Remove logic"

[SourceForge.net]

Bugs item #1538643, was opened at 2006-08-11 14:17
Message generated for change (Tracker Item Submitted) made by Item Submitter
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=109863&aid=1538643&group_id=9863

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Common
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Christian Walther (cwalther)
Assigned to: Nobody/Anonymous (nobody)
Summary: wxMSW & wxMac: "bug in wxMenu::Remove logic"

Initial Comment:
In both wxMSW 2.6.3 and wxMac 2.6.3 (and presumably
also in the current CVS, since the relevant code
doesn't seem to have changed), the code

  wxMenu *menu = new wxMenu();
  wxMenuItem *item = new wxMenuItem(menu, wxID_ANY,
wxT("item"));
  menu->Remove(item);

(which is a simplified version of some real-life code I
have here) triggers an assertion failure in
wxMenu::DoRemove(wxMenuItem *item):

  ...\src\msw\menu.cpp(625): assert "wxAssertFailure"
failed: bug in wxMenu::Remove logic

resp.

  .../src/mac/carbon/menu.cpp(312): assert
"wxAssertFailure" failed: bug in wxMenu::Remove logic

On the line preceding this assertion
(src/msw/menu.cpp:624, src/mac/carbon/menu.cpp:311),
there is a comment "DoRemove() (unlike Remove) can only
be called for existing item!", which is obviously
wrong, since wxMenuBase::Remove(wxMenuItem *item), from
which this function is called, does *not* check whether
the item is actually contained in the menu.

I'm not sure what the expected behavior is when trying
to remove a menu item that is not in the menu (the
documentation doesn't specify anything about it), but
getting an error message that says "bug in remove
logic" probably isn't it. I can imagine two solutions,
of which I'd prefer the latter (though I don't know how
this is handled on other platforms):

- That assertion message should be changed to something
more descriptive, and the documentation should mention
that the user has to check for this condition before
calling Remove().

- The assertion (including the comment above it) should
just be replaced by a silent "if (node == NULL)
return;", and the documentation should mention that
it's OK to call Remove() even when one isn't sure
whether the item is actually in the menu.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=109863&aid=1538643&group_id=9863