Re[2]: [lmi] Untimely "save" dialog [Was: wxmsw-2.9.0 regression: crash when a messagebox should appear]

[Vadim Zeitlin]

On Mon, 09 Mar 2009 13:55:43 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2009-02-24 17:26Z, Greg Chicares wrote:
GC> [...]
GC> > File | Open
GC> > [Files of type:] Illustration (*.ill)
GC> > [File name:] \lmi\src\lmi\sample.ill
GC> > 
GC> > If your local copy of cvs is elsewhere, then change the path
GC> > accordingly: I chose that file because it's in cvs.
GC> > 
GC> > In the "Comments" field, enter "xyz", then press OK. This
GC> > messagebox appears:
GC> > 
GC> > Do you want to save changes to sample.ill?
GC> > Yes   No   Cancel
GC> > 
GC> > I think this arose about a year ago. The "illustration" file type
GC> > is a bit unusual, though I've seen other illustration systems do
GC> > the same thing. "File | Open" (and "File | New") don't immediately
GC> > display a view of the file in an MDI child window. Instead, they
GC> > pop up a dialog that enables that file to be edited. The problem
GC> > is that this dialog appears when OK is pressed. The ancient and
GC> > desired behavior is that no such dialog appear until the user
GC> > closes the window (without having saved the file first).
GC> > 
GC> > I had a vague notion that this regression might be related to
GC> > this 20080323T0117Z change:
GC> > 
GC> > 
http://cvs.savannah.gnu.org/viewvc/lmi/lmi/illustration_document.cpp?r1=1.12&r2=1.13
GC> > 
GC> > and now I've confirmed it with the experimental patch below [1],
GC> > which reverts that change. (I didn't try that patch with wx-2.8.9;
GC> > I tested only the 2009-02-23 wx snapshot, so it's conceivable that
GC> > it wouldn't work with 2.8.9 .) However, I suppose we don't really
GC> > want to revert it, if we can improve the 20080323T0117Z change
GC> > instead.
GC> > 
GC> > Here, I have to ask for help. I remember reviewing that change
GC> > carefully, with a copy of the wx sources in front of me, and
GC> > convincing myself that it was a pure refactoring that couldn't
GC> > produce any observable change in behavior.

 I can explain why the patch results in this change behaviour easily: it's
the same problem as mentioned in a "WX !!" comment before
IllustrationDocument::OnNewDocument() in illustration_document.cpp: wx
calls OnSaveModified() from both wxDocument::OnNewDocument() and
OnOpenDocument() and you reimplement OnNewDocument() there just to avoid
doing this. As long as you also did the same thing with OnOpenDocument()
there was no prompt. But since you don't override OnOpenDocument() any more
and just override DoOpenDocument(), the wx version which calls
OnSaveModified() is used now.

GC> Now I observe a sporadic crash in a similar situation.

 Unfortunately I couldn't reproduce the crash. It might be due to a
difference in exception handling between MSVC and mingw32 but to be honest
I don't see how is this possible. Looking at the backtrace I don't quite
understand what's going on neither, it looks like m_fileMenus in
wxFileHistory got corrupted somehow as otherwise I don't see why would the
call to SetLabel() crash.

GC> Click "OK". An MDI child window has opened; it has no apparent
GC> content, and its title is still "Loading document...".
GC> 
GC> This, I speculate, is the original sin. Loading the document
GC> didn't succeed, but neither did it completely fail, because an
GC> exception escaped. I guess I'd better try to trap it.

 This does need to be fixed, of course -- the window shouldn't be opening
and there should be no prompt. I am working on the patch doing this right
now and will try to post it a.s.a.p. I hoped to attach it to this message
but ran into more problems so I'll need a bit more time but I wanted to
post this already just to let you know I was working on it and to avoid
wasting time on redundant efforts -- as my changes will almost surely
change something (even if they don't fix it) for the crash too.

 Regards,
VZ