document/view framework: OnCloseDocument called twice

[Quentin Cosendey]

Hello,

I'm trying to use the document/view framework, and globally it works fine.
However, in my application, something strange happens when closing a
document/view.

Like in the doc/view sample, I want to allow the user to choose whether
he wants to work in SDI, MDI or tab mode.

In the menubar of all windows, I have added a pretty standard close item
in the file menu, associated with the Ctrl+F4 shortcut.
file->Append(wxID_CLOSE, wxGetStockLabel(wxID_CLOSE) + "\tCtrl+F4");

In tab mode (using wxNotebook), this close item is the only way to close
a tab,
while, when running in SDI mode, I expect it to do exactly the same as
closing the window with Alt+F4.

That's where I observed something strange.

IF I close the window with Alt+F4, the method OnCloseDocument of my
wxDocument subclass is called only once, which is normal. The call comes
from wxView::OnClose.

However, if I close the window with Ctrl+F4, wxDocument::OnCloseDocument
is called twice !
It equally happens if I click on the menu item, too.
ON the first time, the call comes from wxDocManager::CloseDocument, and
the second time from wxView::OnClose.

I don't intercept any event from wxID_CLOSE. Basically my menu
processing method is like this:

void App::OnMenuItemSelected (wxCommandEvent& e) {
switch(e.GetId()) {
case IDM_XXX: ... break;
case IDM_YYY: ... break;
default: e.Skip(); break;
}

IN fact, wxDocument::OnCloseDocument is called twice also when closing a
document with my menu item in MDI and tab mode.

Am I doing something wrong or is that a bug ?
I'm using wxWidgets 3.1.6.

Thank you for your answers.

[Vadim Zeitlin]

On Fri, 8 Jul 2022 16:05:00 +0200 Quentin Cosendey wrote:

QC> Like in the doc/view sample, I want to allow the user to choose whether
QC> he wants to work in SDI, MDI or tab mode.

This is unrelated to the problem being discussed here but, FWIW, I don't
think it's a good idea to allow the user to choose this. The sample does it
just for demonstration purposes, but I've never seen a real application
that would work like this and I think one mode always works better than the
other ones for the concrete application purposes.

QC> In the menubar of all windows, I have added a pretty standard close item
QC> in the file menu, associated with the Ctrl+F4 shortcut.
QC> file->Append(wxID_CLOSE, wxGetStockLabel(wxID_CLOSE) + "\tCtrl+F4");
QC>
QC> In tab mode (using wxNotebook), this close item is the only way to close
QC> a tab, while, when running in SDI mode, I expect it to do exactly the
QC> same as closing the window with Alt+F4.
QC>
QC> That's where I observed something strange.
QC>
QC> IF I close the window with Alt+F4, the method OnCloseDocument of my
QC> wxDocument subclass is called only once, which is normal. The call comes
QC> from wxView::OnClose.
QC>
QC> However, if I close the window with Ctrl+F4, wxDocument::OnCloseDocument
QC> is called twice !
QC> It equally happens if I click on the menu item, too.
QC> ON the first time, the call comes from wxDocManager::CloseDocument, and
QC> the second time from wxView::OnClose.

This definitely looks like a bug. Looking at the code, it's all rather
confusing because wxDocument::Close() calls, via DeleteAllViews(),
wxView::Close() which calls wxView::OnClose() which calls back into
wxDocument::Close() again.

I hope there is no code which actually relies on OnCloseDocument() being
called before the views are destroyed because I just don't see how can we
fix this while preserving this behaviour. If we don't worry about breaking
such code (which really shouldn't exist, but who knows...), then the
changes in https://github.com/wxWidgets/wxWidgets/pull/22627 could be
applied and should fix the problem (and also improve a couple of other
things in the same area).

Regards,
VZ

--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[Quentin Cosendey]

Hello,


VZ>  This is unrelated to the problem being discussed here but, FWIW, I

don't think it's a good idea to allow the user to choose this. The
sample does it just for demonstration purposes, but I've never seen a
real application that would work like this and I think one mode always
works better than the other ones for the concrete application purposes.

I think it's still good at least to let the user choose between tabs and
separate windows.
Personally I often prefer separate windows than tabs when I have the
choice. I have configured my e-mail client and my web browser to do so.
In the opposite, I have the impression that most people prefer tabs.

Maybe you are right for MDI though. As far as I know, it's much less
common nowadays, and quite a windows-specific design.

VZ>  This definitely looks like a bug. Looking at the code, it's all
rather confusing [...]

Yes, the doc itself is a little confusing.

It would have probably been better to have two separate methods, for
example OnClosing and OnClosed, to differentiate more clearly the moment
when you can prevent the close from happening, and when it's already
done, as it's already at other places. For example I think about the
wxEVT_PAGE_CHANGING/PAGE_CHANGED pair of events.
This is much more clear: -ing = the action is going to happen and you
may still cancel it, -ed = the action is done and you can't do anything
about it anymore.
The obscure bool saying whether you should destroy the window could
certainly be removed if it were like that.

In fact, I indeed found this bug because of destruction race condition.
I looks like that, sometimes, the document is indeed destroyed before
the second call
I also sometimes receive an activation event from the window
(wxEVT_ACTIVATE) after the document/view are gone.
I have changed my code to no longer need wxEVT_ACTIVATE, as it isn't
reliable in this regard

IN the meantime, to fight against the two calls, I have just set a "bool
closed" variable both on the document and on the view. If closed==true,
then I return immediately and do nothing.
It works and I no longer have any crash trying to access destroyed objects.

I haven't yet looked at your patch, and what it brings beyond fixing
this bug. I'll surely try it later.
Thank you for your answer.

All the best

[Vadim Zeitlin]

On Tue, 19 Jul 2022 21:51:34 +0200 Quentin Cosendey wrote:

QC> It would have probably been better to have two separate methods, for
QC> example OnClosing and OnClosed

The fundamental problem is that the docview framework can't decide if it's
the view or the document which has the final say, so it asks both of them,
possibly multiple times. Adding more methods is probably not going to help
making things more clear, unfortunately.

QC> I haven't yet looked at your patch, and what it brings beyond fixing
QC> this bug. I'll surely try it later.

Please let me know if you still plan to try it because I'd like to merge
this PR if it works for you.

Thanks,

[arnoldp6525]

Hi,

Can someone help with publishing wxWidgets or direct me to correct information. I am using Visual Studio 2022 Community.

--
Please read https://www.wxwidgets.org/support/mlhowto.htm before posting.
---
You received this message because you are subscribed to the Google Groups "wx-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to wx-users+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/wx-users/e9de4232-9507-4915-11e8-a30f090aa6a6%40bluewin.ch.

[Vadim Zeitlin]

On Sun, 24 Jul 2022 09:49:42 -0700 arnoldp6525 wrote:

a> Can someone help with publishing wxWidgets or direct me to correct
a> information. I am using Visual Studio 2022 Community.

Hello,

If you have a question that you'd like to ask on this mailing list, please
do the following:

- Start a new thread instead of replying to the existing unrelated one.
- Describe the problem or question briefly in the subject (saying that
it's unrelated to something doesn't narrow the possibilities much).
- Formulate the question clearly in the body of your post.

Thanks,