Opened 2 years ago

Closed 23 months ago

#14684 closed defect (fixed)

"Active" and "Selected" wxAuiMDIChildFrame inconsistency

Reported by: wsu Owned by:
Priority: normal Milestone:
Component: wxAui Version: stable-latest
Keywords: wxAuiMDI active selected Cc:
Blocked By: Blocking:
Patch: yes

Description

Suppose a wxAuiMDIParentFrame has a wxAuiMDIChildFrame. If a second wxAuiMDIChildFrame is allocated, and Show(false) is called before Create(), then wxAuiMDIParentFrame has SetActiveChild() called for the second wxAuiMDIChildFrame, but the wxAuiMDIClientWindow::InsertPage() does not call wxAuiMDIChildFrame SetSelectionToWindow() for the second wxAuiMDIChildFrame. This results in the wxAuiMDIParentFrame treating the second wxAuiMDIChildFrame as active, but the wxAuiMDIClientWindow treating the first wxAuiMDIChildFrame as selected.

It would be easy to modify wxAuiMDIParentFrame::SetActiveChild() so that when Show(false) has been called, it skips SetActiveChild() so that the program is in a consistent state with the first wxAuiMDIChildFrame both active and selected. However, I think that is the wrong thing to do. I think the wxAuiMDIParentFrame should be modified so that SetActiveChild() and GetActiveChild() are convenience functions wrapping the wxAuiMDIClientWindow SetSelection() and GetSelection(). This would prevent any other situation where the active and selected children are different.

I have included a patch to make this change.

(This was originally discussed in http://groups.google.com/d/msg/wx-dev/kBBJChNYumA/oOjq0p8CeaMJ)

Attachments (3)

wxAuiMDI rewrite Active.patch download (4.3 KB) - added by wsu 2 years ago.
wxAuiMDI rewrite Active.2.patch download (5.7 KB) - added by wsu 23 months ago.
fix wxAuiMDIParent creation gpf.patch download (574 bytes) - added by wsu 23 months ago.

Download all attachments as: .zip

Change History (10)

Changed 2 years ago by wsu

comment:1 Changed 2 years ago by vadz

Thanks, this definitely makes sense to me. But, as usual, I have some questions about the patch:

  1. Why do we need to use wxDynamicCast(wxMDIChildFrame) the return value of GetActiveChild() which had already used wxStaticCast(wxMDIChildFrame) inside it? This doesn't seem to make much sense.
  2. What is the assert (involving m_activateOnCreate) added by this patch supposed to check, exactly? Could you please add either a message or a comment to it and, perhaps, simplify its condition to make it more clear?
  3. What is the "comment in ctor" you refer to in Show()? Why doesn't it make sense to call it after creation?

TIA!

Changed 23 months ago by wsu

comment:2 Changed 23 months ago by wsu

I have posted a new version of the patch.

  1. I don't think the wxDynamicCast was redundant, but that was because there were two not-quite-the-same versions of the same code. I have merged them now.

2 and 3. These are surprises (at least to me) that tripped me up when working on this code. I have added more extensive comments. Please see if you find this version more helpful.

comment:3 Changed 23 months ago by vadz

Thanks, the explanations do help. I'll commit this soon.

comment:4 Changed 23 months ago by VZ

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

(In [72556]) Merge "selected" and "active" child in wxAuiMDIParentFrame.

They are one and the same thing and so just make them really synonymous
instead of (unsuccessfully) trying to keep them synchronized.

Closes #14684.

comment:5 Changed 23 months ago by wsu

  • Resolution fixed deleted
  • Status changed from closed to reopened

Big Muscle has pointed out that this is causing a crash when processing wxAuiMDIParentFrame::Create(). This patch fixes that. (I didn't see this in my application because my class derived from wxAuiMDIParentFrame has overridden ProcessEvent(). Unfortunately, there is no wxAuiMDI sample.)

comment:6 Changed 23 months ago by vadz

Thanks, I'll apply the patch but I wonder if we should test for GetClientWindow() != NULL in other places, e.g. SetActiveChild()? I'll probably add a test there too, just in case...

comment:7 Changed 23 months ago by VZ

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

(In [72601]) Test for wxAuiMDIClientWindow being non-NULL before using it.

wxAuiMDIParentFrame::GetActiveChild() may be called before the client window
is created, don't crash in this case but just return NULL.

Closes #14684.

Note: See TracTickets for help on using tickets.