tdf#156546 sw: fix infinite loop in SwUndoInsert::RedoImpl()

The problem is that SwUndoSaveContent::MovePtBackward() sets the point
of a shell cursor to the document body's start node, which is not a
valid position for a shell cursor; FindParentText() then loops forever.

The purpose of this appears to be to move the point temporarily
somewhere where subsequent inserting operations won't move it further,
so that it can be restored to the start of the inserted stuff.

Refactor a bit to use a temporary SwNodeIndex instead, which should work
as nothing should delete the node it's pointing to.

(regression from commit d81379db730a163c5ff75d4f3a3cddbd7b5eddda)

Change-Id: I471bcced1741c77c07239ed124d4fd39ff7a7515
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155227
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
(cherry picked from commit 2d96d69322ac18f53668b75397c8587f94cd043b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155239
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

4 files changed, 19 insertions, 23 deletions

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index f95a3aa78bfc..9feaec0521b7 100644
--- a/sw/inc/undobj.hxx
+++ b/sw/inc/undobj.hxx
@@ -179,12 +179,11 @@ protected:
                           const SwNodeOffset* pEndNdIdx = nullptr,
                           bool bForceCreateFrames = false);
 
-    // These two methods move the SPoint back/forth from PaM. With it
-    // a range can be spanned for Undo/Redo. (In this case the SPoint
-    // is before the manipulated range!!)
-    // The flag indicates if there is content before the SPoint.
-    static bool MovePtBackward( SwPaM& rPam );
-    static void MovePtForward( SwPaM& rPam, bool bMvBkwrd );
+    // These two methods save and restore the Point of PaM.
+    // If the point cannot be moved, a "backup" is created on the previous node.
+    // Either way, it will not be moved by inserting at its original position.
+    static ::std::optional<SwNodeIndex> MovePtBackward(SwPaM& rPam);
+    static void MovePtForward(SwPaM& rPam, ::std::optional<SwNodeIndex> && oMvBkwrd);
 
     // Before moving stuff into UndoNodes-Array care has to be taken that
     // the content-bearing attributes are removed from the nodes-array.
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index 5e80183a1b38..0f6138c75cdb 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -824,29 +824,26 @@ void SwUndoSaveContent::MoveFromUndoNds( SwDoc& rDoc, SwNodeOffset nNodeIdx,
     }
 }
 
-// These two methods move the Point of Pam backwards/forwards. With that, one
-// can span an area for a Undo/Redo. (The Point is then positioned in front of
-// the area to manipulate!)
-// The flag indicates if there is still content in front of Point.
-bool SwUndoSaveContent::MovePtBackward( SwPaM& rPam )
+// These two methods save and restore the Point of PaM.
+// If the point cannot be moved, a "backup" is created on the previous node.
+// Either way, returned, inserting at its original position will not move it.
+::std::optional<SwNodeIndex> SwUndoSaveContent::MovePtBackward(SwPaM & rPam)
 {
     rPam.SetMark();
     if( rPam.Move( fnMoveBackward ))
-        return true;
+        return {};
 
-    // If there is no content onwards, set Point simply to the previous position
-    // (Node and Content, so that Content will be detached!)
-    rPam.GetPoint()->Adjust(SwNodeOffset(-1));
-    return false;
+    return { SwNodeIndex(rPam.GetPoint()->GetNode(), -1) };
 }
 
-void SwUndoSaveContent::MovePtForward( SwPaM& rPam, bool bMvBkwrd )
+void SwUndoSaveContent::MovePtForward(SwPaM& rPam, ::std::optional<SwNodeIndex> && oMvBkwrd)
 {
     // Was there content before this position?
-    if( bMvBkwrd )
+    if (!oMvBkwrd)
         rPam.Move( fnMoveForward );
     else
     {
+        *rPam.GetPoint() = SwPosition(*oMvBkwrd);
         rPam.GetPoint()->Adjust(SwNodeOffset(1));
         SwContentNode* pCNd = rPam.GetPointContentNode();
         if( !pCNd )
diff --git a/sw/source/core/undo/unins.cxx b/sw/source/core/undo/unins.cxx
index 29eb0408d5bc..25ba85df20cd 100644
--- a/sw/source/core/undo/unins.cxx
+++ b/sw/source/core/undo/unins.cxx
@@ -321,7 +321,7 @@ void SwUndoInsert::RedoImpl(::sw::UndoRedoContext & rContext)
 
         if( m_nLen )
         {
-            const bool bMvBkwrd = MovePtBackward( *pPam );
+            ::std::optional<SwNodeIndex> oMvBkwrd = MovePtBackward(*pPam);
 
             if (maText)
             {
@@ -348,7 +348,7 @@ void SwUndoInsert::RedoImpl(::sw::UndoRedoContext & rContext)
             m_nNode = pPam->GetMark()->GetNodeIndex();
             m_nContent = pPam->GetMark()->GetContentIndex();
 
-            MovePtForward( *pPam, bMvBkwrd );
+            MovePtForward(*pPam, ::std::move(oMvBkwrd));
             pPam->Exchange();
             if( m_pRedlData && IDocumentRedlineAccess::IsRedlineOn( GetRedlineFlags() ))
             {
diff --git a/sw/source/core/undo/untblk.cxx b/sw/source/core/undo/untblk.cxx
index 29b49e17a485..fd5c3239c042 100644
--- a/sw/source/core/undo/untblk.cxx
+++ b/sw/source/core/undo/untblk.cxx
@@ -389,15 +389,15 @@ void SwUndoInserts::RedoImpl(::sw::UndoRedoContext & rContext)
         auto const pFlysAtInsPos(sw::GetFlysAnchoredAt(rDoc,
             rPam.GetPoint()->GetNodeIndex()));
 
-        const bool bMvBkwrd = MovePtBackward(rPam);
+        ::std::optional<SwNodeIndex> oMvBkwrd = MovePtBackward(rPam);
 
         // re-insert content again (first detach m_oUndoNodeIndex!)
         SwNodeOffset const nMvNd = m_oUndoNodeIndex->GetIndex();
         m_oUndoNodeIndex.reset();
         MoveFromUndoNds(rDoc, nMvNd, *rPam.GetMark());
-        if (m_nDeleteTextNodes != SwNodeOffset(0))
+        if (m_nDeleteTextNodes != SwNodeOffset(0) || oMvBkwrd)
         {
-            MovePtForward(rPam, bMvBkwrd);
+            MovePtForward(rPam, ::std::move(oMvBkwrd));
         }
         rPam.Exchange();