Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#15663 closed defect (fixed)

wxMDIParentFrame::SetWindowMenu() erroneously calls wxMDIParentFrame::AddWindowMenu()

Reported by: laro Owned by:
Priority: normal Milestone:
Component: wxMSW Version: 3.0.0
Keywords: wxMDIParentFrame, SetWindowMenu, AddWindowMenu, AddMDIChild Cc:
Blocked By: Blocking:
Patch: yes

Description

AddWindowMenu() should be called only if the number of MDIChildFrames is more than one. Curently AddWindowMenu() crashes when called for the second time in wxMDIParentFrame::AddMDIChild().

BTW, I don't really like hiding the window menu just because there (currently) is no window visible.

Attachments (2)

mdi.cpp.patch download (357 bytes) - added by laro 12 months ago.
samples_mdi_mdi.cpp.patch download (1.5 KB) - added by laro 12 months ago.
Modified sample to show the bug.

Download all attachments as: .zip

Change History (7)

Changed 12 months ago by laro

comment:1 follow-up: Changed 12 months ago by vadz

  • Status changed from new to infoneeded_new

This seems to be inconsistent with wxMDIParentFrame::AddMDIChild(), so I don't think it's the right thing to do. Should this > 1 be > 0 perhaps?

When exactly does it crash and how, could you please provide a patch reproducing the crash in the MDI sample?

Finally, I think it's the native convention to only show the "Window" menu when there are any children, so we're just following it.

comment:2 in reply to: ↑ 1 Changed 12 months ago by laro

  • Status changed from infoneeded_new to new
  • Version changed from 3.0.0-rc2 to 3.0.0

Replying to vadz:

This seems to be inconsistent with wxMDIParentFrame::AddMDIChild(), so I don't think it's the right thing to do. Should this > 1 be > 0 perhaps?

Yes, you are right.

When exactly does it crash and how, could you please provide a patch reproducing the crash in the MDI sample?

  1. An assertion is violated during SetWindowMenu(). (I think I just ignored this up to now.)

..\..\src\common\menucmn.cpp(701): assert "m_menuBar" failed in wxMenuBase::Detach(): detaching unattached menu?

Call stack:
[00] wxMDIParentFrame::RemoveWindowMenu wxwidgets\src\msw\mdi.cpp:344
[01] wxMDIParentFrame::SetWindowMenu wxwidgets\src\msw\mdi.cpp:381
[02] MyFrame::MyFrame wxwidgets\samples\mdi\mdi.cpp:157
[03] MyApp::OnInit wxwidgets\samples\mdi\mdi.cpp:121
[04] wxAppConsoleBase::CallOnInit

  1. The Window menu appears on SetWindowMenu() while it should be hidden.
  1. An assertion is violated later on when the user creates the first MDIChildFrame:

..\..\src\common\menucmn.cpp(693): assert "!m_menuBar" failed in wxMenuBase::Attach(): attaching menu twice?

Call stack:
[00] wxMDIParentFrame::AddWindowMenu \wxwidgets\src\msw\mdi.cpp:332
[01] wxMDIParentFrame::AddMDIChild \wxwidgets\src\msw\mdi.cpp:288
[02] wxMDIChildFrame::Create \wxwidgets\src\msw\mdi.cpp:853
[03] wxMDIChildFrame::wxMDIChildFrame \wxwidgets\include\wx\msw\mdi.h:174
[04] MyChild::MyChild \wxwidgets\samples\mdi\mdi.cpp:406
[05] MyFrame::OnNewWindow \wxwidgets\samples\mdi\mdi.cpp:251

Changed 12 months ago by laro

Modified sample to show the bug.

comment:3 Changed 12 months ago by vadz

  • Status changed from new to confirmed

Thank you, I do see (and understand) the problem now and your fix is indeed correct, I'll commit it soon.

comment:4 Changed 12 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [75242]) Fix replacing "Window" menu in wxMDIParentFrame in wxMSW.

When setting a new "Window" menu (as opposed to just modifying the existing
one which did work correctly), we shouldn't show it immediately in the menu
bar if there are no MDI children, this results in wrong UI and assert
failures.

Closes #15663.

comment:5 Changed 12 months ago by VZ

(In [75244]) Fix replacing "Window" menu in wxMDIParentFrame in wxMSW.

When setting a new "Window" menu (as opposed to just modifying the existing
one which did work correctly), we shouldn't show it immediately in the menu
bar if there are no MDI children, this results in wrong UI and assert
failures.

Closes #15663.

Note: See TracTickets for help on using tickets.