sw/source/core/undo/untblk.cxx | 73 +++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 17 deletions(-)
New commits: commit b9d14fbad622a05320eb90211ae2ba89edfb8148 Author: Fyodor Yemelyanenko <fyodo...@hotmail.com> Date: Thu Nov 16 17:06:28 2017 +1000 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 <mst...@redhat.com> Tested-by: Michael Stahl <mst...@redhat.com> (cherry picked from commit 70483089425d5bb22e036867290e06a6fc8d99fb) Reviewed-on: https://gerrit.libreoffice.org/48697 Tested-by: Jenkins <c...@libreoffice.org> 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 _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits