Merge branch 'docview-onclosedoc'

Improvements to closing wxDocument: don't call OnCloseDocument() twice.

See #22627.

docs/changes.txt

@@ -15,6 +15,11 @@ Changes in behaviour not resulting in compilation errors
   used by wxThread::Delete() and Wait() by default, has changed from
   wxTHREAD_WAIT_YIELD to wxTHREAD_WAIT_BLOCK for safety and consistency.
 
+- wxDocument::OnCloseDocument() was called twice in previous versions when
+  closing the document from the menu. Now it is only called once and after
+  destroying all the existing document views. If you overrode this function,
+  please check that you don't rely on any views existing when it's called.
+
 
 Changes in behaviour which may result in build errors
 -----------------------------------------------------

include/wx/docview.h

@@ -117,6 +117,10 @@ public:
     // modified to false)
     virtual bool OnSaveModified();
 
+    // Similar to OnSaveModified() but doesn't allow the user to prevent the
+    // document from closing as it will be closed unconditionally.
+    virtual void OnSaveBeforeForceClose();
+
     // if you override, remember to call the default
     // implementation (wxDocument::OnChangeFilename)
     virtual void OnChangeFilename(bool notifyViews);
@@ -181,6 +185,12 @@ public:
     // part of the parent document and not a disk file as usual.
     bool IsChildDocument() const { return m_documentParent != NULL; }
 
+    // Ask the user if the document should be saved if it's modified and save
+    // it if necessary.
+    //
+    // Returns false if the user cancelled closing or if saving failed.
+    bool CanClose();
+
 protected:
     wxList                m_documentViews;
     wxString              m_documentFile;

interface/wx/docview.h

@@ -1472,10 +1472,12 @@ public:
         case since wxWidgets 2.9.0.
 
         Returning @false from this function prevents the document from closing.
-        The default implementation does this if the document is modified and
-        the user didn't confirm discarding the modifications to it.
-
-        Return @true to allow the document to be closed.
+        Note that there is no need to ask the user if the changes to the
+        document should be saved, as this was already checked by
+        OnSaveModified() by the time this function is called, if necessary, and
+        so, typically, this function should always return @true to allow the
+        document to be closed, as leaving it open after asking the user about
+        saving the changes would be confusing.
     */
     virtual bool OnCloseDocument();
 
@@ -1542,6 +1544,19 @@ public:
     */
     virtual bool OnSaveModified();
 
+    /**
+        This function is called when a document is forced to close.
+
+        The default implementation asks the user whether to save the changes
+        but, unlike OnSaveModified(), does not allow to cancel closing.
+
+        The document is force closed when wxDocManager::CloseDocument() is
+        called with its @c force argument set to @true.
+
+        @since 3.3.0
+     */
+    virtual void OnSaveBeforeForceClose();
+
     /**
         Removes the view from the document's list of views.
 

samples/docview/docview.cpp

@@ -75,6 +75,7 @@ wxIMPLEMENT_APP(MyApp);
 
 wxBEGIN_EVENT_TABLE(MyApp, wxApp)
     EVT_MENU(wxID_ABOUT, MyApp::OnAbout)
+    EVT_MENU(wxID_CLEAR, MyApp::OnForceCloseAll)
 wxEND_EVENT_TABLE()
 
 MyApp::MyApp()
@@ -315,6 +316,7 @@ void MyApp::AppendDocumentFileCommands(wxMenu *menu, bool supportsPrinting)
     menu->Append(wxID_SAVE);
     menu->Append(wxID_SAVEAS);
     menu->Append(wxID_REVERT, _("Re&vert..."));
+    menu->Append(wxID_CLEAR, "&Force close all");
 
     if ( supportsPrinting )
     {
@@ -437,6 +439,13 @@ wxFrame *MyApp::CreateChildFrame(wxView *view, bool isCanvas)
     return subframe;
 }
 
+void MyApp::OnForceCloseAll(wxCommandEvent& WXUNUSED(event))
+{
+    // Pass "true" here to force closing just for testing this functionality,
+    // there is no real reason to force the issue here.
+    wxDocManager::GetDocumentManager()->CloseDocuments(true);
+}
+
 void MyApp::OnAbout(wxCommandEvent& WXUNUSED(event))
 {
     wxString modeName;

samples/docview/docview.h

@@ -71,6 +71,9 @@ private:
     void CreateMenuBarForFrame(wxFrame *frame, wxMenu *file, wxMenu *edit);
 
 
+    // force close all windows
+    void OnForceCloseAll(wxCommandEvent& event);
+
     // show the about box: as we can have different frames it's more
     // convenient, even if somewhat less usual, to handle this in the
     // application object itself

src/common/docview.cpp

@@ -144,15 +144,14 @@ wxDocument::~wxDocument()
     //DeleteAllViews();
 }
 
-bool wxDocument::Close()
+bool wxDocument::CanClose()
 {
     if ( !OnSaveModified() )
         return false;
 
     // When the parent document closes, its children must be closed as well as
-    // they can't exist without the parent.
+    // they can't exist without the parent, so ask them too.
 
-    // As usual, first check if all children can be closed.
     DocsList::const_iterator it = m_childDocuments.begin();
     for ( DocsList::const_iterator end = m_childDocuments.end(); it != end; ++it )
     {
@@ -163,6 +162,15 @@ bool wxDocument::Close()
         }
     }
 
+    return true;
+}
+
+bool wxDocument::Close()
+{
+    // First check if this document itself and all its children can be closed.
+    if ( !CanClose() )
+        return false;
+
     // Now that they all did, do close them: as m_childDocuments is modified as
     // we iterate over it, don't use the usual for-style iteration here.
     while ( !m_childDocuments.empty() )
@@ -498,26 +506,34 @@ bool wxDocument::OnSaveModified()
 {
     if ( IsModified() )
     {
-        switch ( wxMessageBox
-                 (
-                    wxString::Format
-                    (
-                     _("Do you want to save changes to %s?"),
-                     GetUserReadableName()
-                    ),
-                    wxTheApp->GetAppDisplayName(),
-                    wxYES_NO | wxCANCEL | wxICON_QUESTION | wxCENTRE,
-                    GetDocumentWindow()
-                 ) )
+        wxMessageDialog dialogSave
+            (
+                GetDocumentWindow(),
+                wxString::Format
+                (
+                    _("Do you want to save changes to %s?"),
+                    GetUserReadableName()
+                ),
+                wxTheApp->GetAppDisplayName(),
+                wxYES_NO | wxCANCEL | wxICON_QUESTION | wxCENTRE
+            );
+        dialogSave.SetYesNoCancelLabels
+            (
+                _("&Save"),
+                _("&Discard changes"),
+                _("Do&n't close")
+            );
+
+        switch ( dialogSave.ShowModal() )
         {
-            case wxNO:
+            case wxID_NO:
                 Modify(false);
                 break;
 
-            case wxYES:
+            case wxID_YES:
                 return Save();
 
-            case wxCANCEL:
+            case wxID_CANCEL:
                 return false;
         }
     }
@@ -525,6 +541,50 @@ bool wxDocument::OnSaveModified()
     return true;
 }
 
+void wxDocument::OnSaveBeforeForceClose()
+{
+    if ( !IsModified() )
+        return;
+
+    wxMessageDialog dialogSave
+        (
+            GetDocumentWindow(),
+            wxString::Format
+            (
+                _("Do you want to save changes to %s before closing it?"),
+                GetUserReadableName()
+            ),
+            wxTheApp->GetAppDisplayName(),
+            wxYES_NO | wxICON_QUESTION | wxCENTRE
+        );
+    dialogSave.SetExtendedMessage(_("The document must be closed."));
+    dialogSave.SetYesNoLabels(_("&Save"), _("&Discard changes"));
+
+    if ( dialogSave.ShowModal() == wxID_YES )
+    {
+        while ( !Save() )
+        {
+            wxMessageDialog dialogRetry
+                (
+                    GetDocumentWindow(),
+                    wxString::Format
+                    (
+                        _("Saving %s failed, would you like to retry?"),
+                        GetUserReadableName()
+                    ),
+                    wxTheApp->GetAppDisplayName(),
+                    wxYES_NO | wxICON_ERROR | wxCENTRE
+                );
+            dialogRetry.SetYesNoLabels(_("Retry"), _("Discard changes"));
+
+            if ( dialogRetry.ShowModal() != wxID_YES )
+                break;
+        }
+    }
+
+    Modify(false);
+}
+
 bool wxDocument::Draw(wxDC& WXUNUSED(context))
 {
     return true;
@@ -975,14 +1035,19 @@ wxDocManager::~wxDocManager()
 // closes the specified document
 bool wxDocManager::CloseDocument(wxDocument* doc, bool force)
 {
-    if ( !doc->Close() && !force )
-        return false;
+    if ( force )
+    {
+        // We need to close, but at least ask the user if the document should
+        // be saved before doing it.
+        doc->OnSaveBeforeForceClose();
+    }
+    else // Allow the user to cancel closing too.
+    {
+        if ( !doc->CanClose() )
+            return false;
+    }
 
-    // To really force the document to close, we must ensure that it isn't
-    // modified, otherwise it would ask the user about whether it should be
-    // destroyed (again, it had been already done by Close() above) and might
-    // not destroy it at all, while we must do it here.
-    doc->Modify(false);
+    // Note that by now the document is certain not to be modified any longer.
 
     // Implicitly deletes the document when
     // the last view is deleted