sw/inc/undobj.hxx | 11 +++++------ sw/source/core/undo/undobj.cxx | 21 +++++++++------------ sw/source/core/undo/unins.cxx | 4 ++-- sw/source/core/undo/untblk.cxx | 6 +++--- 4 files changed, 19 insertions(+), 23 deletions(-)
New commits: commit 2d96d69322ac18f53668b75397c8587f94cd043b Author: Michael Stahl <michael.st...@allotropia.de> AuthorDate: Wed Aug 2 13:26:39 2023 +0200 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Wed Aug 2 20:40:37 2023 +0200 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.st...@allotropia.de> 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 c0a8427b0361..dce7f3eb2724 100644 --- a/sw/source/core/undo/undobj.cxx +++ b/sw/source/core/undo/undobj.cxx @@ -860,29 +860,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 38ac9d49c65b..0ac95d0c192f 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();