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

[Greg Chicares]

On 2009-02-24 16:26Z, Greg Chicares wrote:
> Load lmi, then do
>   File | New | Illustration
> but do not yet press Enter or click OK.
> 
> The "Extra policy fee" field contains '0' by default.
> Change that to '0,1000', which is an error that lmi diagnoses.
> Press Enter or click OK.
> 
> First, this messagebox appears [0]:
> 
> Do you want to save changes to unnamed?
> Yes   No   Cancel
[...]
> [0] "First, this messagebox appears"
> 
> The "Do you want to save changes to unnamed?" messagebox is
> perplexing to users. It is not a wx regression; it is an
> lmi regression that occurred months ago but has not been
> reported previously.

I may have conflated an old lmi regression with a new wx
regression, so let's separate them. Use this testcase instead:

File | Open
[Files of type:] Illustration (*.ill)
[File name:] \lmi\src\lmi\sample.ill

If your local copy of cvs is elsewhere, then change the path
accordingly: I chose that file because it's in cvs.

In the "Comments" field, enter "xyz", then press OK. This
messagebox appears:

Do you want to save changes to sample.ill?
Yes   No   Cancel

I think this arose about a year ago. The "illustration" file type
is a bit unusual, though I've seen other illustration systems do
the same thing. "File | Open" (and "File | New") don't immediately
display a view of the file in an MDI child window. Instead, they
pop up a dialog that enables that file to be edited. The problem
is that this dialog appears when OK is pressed. The ancient and
desired behavior is that no such dialog appear until the user
closes the window (without having saved the file first).

I had a vague notion that this regression might be related to
this 20080323T0117Z change:

http://cvs.savannah.gnu.org/viewvc/lmi/lmi/illustration_document.cpp?r1=1.12&r2=1.13

and now I've confirmed it with the experimental patch below [1],
which reverts that change. (I didn't try that patch with wx-2.8.9;
I tested only the 2009-02-23 wx snapshot, so it's conceivable that
it wouldn't work with 2.8.9 .) However, I suppose we don't really
want to revert it, if we can improve the 20080323T0117Z change
instead.

Here, I have to ask for help. I remember reviewing that change
carefully, with a copy of the wx sources in front of me, and
convincing myself that it was a pure refactoring that couldn't
produce any observable change in behavior.

---------

[1] "the experimental patch below"

An attempt to reverse-apply patches taken directly from
  
http://cvs.savannah.gnu.org/viewvc/lmi/lmi/illustration_document.hpp?r1=1.10&r2=1.11
  
http://cvs.savannah.gnu.org/viewvc/lmi/lmi/illustration_document.cpp?r1=1.12&r2=1.13
failed, so I wrote the following changes manually. (I didn't
attempt to rectify the comments that contradict this reversion.)

Index: illustration_document.cpp
===================================================================
RCS file: /sources/lmi/lmi/illustration_document.cpp,v
retrieving revision 1.23
diff -U 3 -r1.23 illustration_document.cpp
--- illustration_document.cpp   27 Dec 2008 02:56:44 -0000      1.23
+++ illustration_document.cpp   24 Feb 2009 16:46:11 -0000
@@ -145,8 +145,14 @@
 /// Override DoOpenDocument() instead of OnOpenDocument(): the latter
 /// doesn't permit customizing its diagnostic messages.

-bool IllustrationDocument::DoOpenDocument(wxString const& filename)
+bool IllustrationDocument::OnOpenDocument(wxString const& filename)
 {
+    SetFilename(filename, true);
+    // WX !! This necessary function is undocumented. It must be
+    // called here; otherwise, wxView::OnSave() pops up an annoying
+    // 'Save As' dialog downstream when the file loaded here is saved.
+    SetDocumentSaved();
+    UpdateAllViews();
     return true;
 }

@@ -155,7 +161,7 @@
 /// Override DoSaveDocument() instead of OnSaveDocument(): the latter
 /// doesn't permit customizing its diagnostic messages.

-bool IllustrationDocument::DoSaveDocument(wxString const& filename)
+bool IllustrationDocument::OnSaveDocument(wxString const& filename)
 {
     if(is_phony_)
         {
@@ -165,6 +171,7 @@

     std::ofstream ofs(filename.c_str(), ios_out_trunc_binary());
     doc_.write(ofs);
+    Modify(false);
     if(!ofs)
         {
         warning() << "Unable to save '" << filename << "'." << LMI_FLUSH;
Index: illustration_document.hpp
===================================================================
RCS file: /sources/lmi/lmi/illustration_document.hpp,v
retrieving revision 1.15
diff -U 3 -r1.15 illustration_document.hpp
--- illustration_document.hpp   27 Dec 2008 02:56:44 -0000      1.15
+++ illustration_document.hpp   24 Feb 2009 16:46:11 -0000
@@ -67,8 +67,8 @@
     // wxDocument overrides.
     virtual bool OnCreate(wxString const& filename, long int flags);
     virtual bool OnNewDocument();
-    virtual bool DoOpenDocument(wxString const& filename);
-    virtual bool DoSaveDocument(wxString const& filename);
+    virtual bool OnOpenDocument(wxString const& filename);
+    virtual bool OnSaveDocument(wxString const& filename);

     single_cell_document doc_;