tdf#94225 - Writer crashes on Undo N times

when document contains flys anchored to paragraph.

Change-Id: I628ef10b1e7817b554afee5e3c8733c1a128b201
Reviewed-on: https://gerrit.libreoffice.org/44800
Reviewed-by: Michael Stahl <mstahl@redhat.com>
Tested-by: Michael Stahl <mstahl@redhat.com>
(cherry picked from commit 70483089425d5bb22e036867290e06a6fc8d99fb)
Reviewed-on: https://gerrit.libreoffice.org/48697
Tested-by: Jenkins <ci@libreoffice.org>

1 files changed, 56 insertions, 17 deletions

diff --git a/sw/source/core/undo/untblk.cxx b/sw/source/core/undo/untblk.cxx
index 9b44e00522ea..d00210353875 100644
--- a/sw/source/core/undo/untblk.cxx
+++ b/sw/source/core/undo/untblk.cxx
@@ -49,22 +49,24 @@ SwUndoInserts::SwUndoInserts( SwUndoId nUndoId, const SwPaM& rPam )
         if( pTextNd->HasSwAttrSet() )
             pHistory->CopyFormatAttr( *pTextNd->GetpSwAttrSet(), nSttNode );
 
-        if( !nSttContent )    // than take the Flys along
+        // We may have some flys anchored to paragraph where we inserting.
+        // These flys will be saved in pFrameFormats array (only flys which exist BEFORE insertion!)
+        // Then in SwUndoInserts::SetInsertRange the flys saved in pFrameFormats will NOT create Undos.
+        // m_FlyUndos will only be filled with newly inserted flys.
+
+        const size_t nArrLen = pDoc->GetSpzFrameFormats()->size();
+        for( size_t n = 0; n < nArrLen; ++n )
         {
-            const size_t nArrLen = pDoc->GetSpzFrameFormats()->size();
-            for( size_t n = 0; n < nArrLen; ++n )
+            SwFrameFormat* pFormat = (*pDoc->GetSpzFrameFormats())[n];
+            SwFormatAnchor const*const  pAnchor = &pFormat->GetAnchor();
+            const SwPosition* pAPos = pAnchor->GetContentAnchor();
+            if (pAPos &&
+                (pAnchor->GetAnchorId() == RndStdIds::FLY_AT_PARA) &&
+                 nSttNode == pAPos->nNode.GetIndex() )
             {
-                SwFrameFormat* pFormat = (*pDoc->GetSpzFrameFormats())[n];
-                SwFormatAnchor const*const  pAnchor = &pFormat->GetAnchor();
-                const SwPosition* pAPos = pAnchor->GetContentAnchor();
-                if (pAPos &&
-                    (pAnchor->GetAnchorId() == RndStdIds::FLY_AT_PARA) &&
-                     nSttNode == pAPos->nNode.GetIndex() )
-                {
-                    if( !pFrameFormats )
-                        pFrameFormats = new std::vector<SwFrameFormat*>;
-                    pFrameFormats->push_back( pFormat );
-                }
+                if( !pFrameFormats )
+                    pFrameFormats = new std::vector<SwFrameFormat*>;
+                pFrameFormats->push_back( pFormat );
             }
         }
     }
@@ -76,7 +78,18 @@ SwUndoInserts::SwUndoInserts( SwUndoId nUndoId, const SwPaM& rPam )
     }
 }
 
-// set destination after reading input
+// This method does two things:
+// 1. Adjusts SwUndoRng members, required for Undo.
+//  Members are:
+//  SwUndoRng::nSttNode - all nodes starting from this node will be deleted during Undo (in SwUndoInserts::UndoImpl)
+//  SwUndoRng::nSttContent - corresponding content index in SwUndoRng::nSttNode
+//  SwUndoRng::nEndNode - end node for deletion
+//  SwUndoRng::nEndContent - end content index
+// All these members are filled in during construction of SwUndoInserts instance, and can be adjusted using this method
+//
+// 2. Fills in m_FlyUndos array with flys anchored ONLY to first and last paragraphs (first == rPam.Start(), last == rPam.End())
+//  Flys, anchored to any paragraph, but not first and last, are handled by DelContentIndex (see SwUndoInserts::UndoImpl) and are not stored in m_FlyUndos.
+
 void SwUndoInserts::SetInsertRange( const SwPaM& rPam, bool bScanFlys,
                                     bool bSttIsTextNd )
 {
@@ -100,7 +113,9 @@ void SwUndoInserts::SetInsertRange( const SwPaM& rPam, bool bScanFlys,
         }
     }
 
-    if( bScanFlys && !nSttContent )
+    // Fill m_FlyUndos with flys anchored to first and last paragraphs
+
+    if( bScanFlys)
     {
         // than collect all new Flys
         SwDoc* pDoc = rPam.GetDoc();
@@ -112,7 +127,7 @@ void SwUndoInserts::SetInsertRange( const SwPaM& rPam, bool bScanFlys,
             SwPosition const*const pAPos = pAnchor->GetContentAnchor();
             if (pAPos &&
                 (pAnchor->GetAnchorId() == RndStdIds::FLY_AT_PARA) &&
-                nSttNode == pAPos->nNode.GetIndex() )
+                (nSttNode == pAPos->nNode.GetIndex() || nEndNode == pAPos->nNode.GetIndex()))
             {
                 std::vector<SwFrameFormat*>::iterator it;
                 if( !pFrameFormats ||
@@ -145,6 +160,27 @@ SwUndoInserts::~SwUndoInserts()
     delete pRedlData;
 }
 
+// Undo Insert operation
+//  It's important to note that Undo stores absolute node indexes. I.e. if during insertion, you insert nodes 31 to 33,
+//  during Undo nodes with indices from 31 to 33 will be deleted. Undo doesn't check that nodes 31 to 33 are the same nodes which were inserted.
+//  It just deletes them.
+//  This may seem as bad programming practice, but Undo actions are strongly ordered. If you change your document in some way, a new Undo action is added.
+//  During Undo most recent actions will be executed first. So during execution of particular Undo action indices will be correct.
+//  But storing absolute indices leads to crashes if some action in Undo fails to roll back some modifications.
+
+//  Has following main steps:
+//  1. DelContentIndex to delete footnotes, flys, bookmarks (see comment for this function)
+//     Deleted flys are stored in pHistory array.
+//     First and last paragraphs flys are handled later in this function! They are not deleted by DelContentIndex!
+//     For flys anchored to last paragraph, DelContentIndex re-anchors them to the last paragraph that will remain after Undo.
+//     This is not fully correct, as everything between nSttNode and nEndNode should be deleted (these nodes marks range of inserted nodes).
+//     But due to bug in paste (probably there), during paste all flys are anchored to last paragraph (see https://bugs.documentfoundation.org/show_bug.cgi?id=94225#c38).
+//     So they should be re-anchored.
+//  2. MoveToUndoNds moves nodes to Undo nodes array and removes them from document.
+//  3. m_FlyUndos removes flys anchored to first and last paragraph in Undo range. This array may be empty.
+//  4. Lastly (starting from if(pTextNode)), text from last paragraph is joined to last remaining paragraph and FormatColl for last paragraph is restored.
+//     Format coll for last paragraph is removed during execution of UndoImpl
+
 void SwUndoInserts::UndoImpl(::sw::UndoRedoContext & rContext)
 {
     SwDoc& rDoc = rContext.GetDoc();
@@ -239,6 +275,9 @@ void SwUndoInserts::UndoImpl(::sw::UndoRedoContext & rContext)
     }
 }
 
+// See SwUndoInserts::UndoImpl comments
+// All actions here should be done in reverse order to what is done in SwUndoInserts::UndoImpl!
+
 void SwUndoInserts::RedoImpl(::sw::UndoRedoContext & rContext)
 {
     // position cursor onto REDO section