sw/qa/core/doc/doc.cxx | 36 ++++++++++++++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 16 ++--------- sw/source/core/inc/UndoRedline.hxx | 6 ++-- sw/source/core/undo/unredln.cxx | 21 +++++++++++---- 4 files changed, 59 insertions(+), 20 deletions(-)
New commits: commit 6dd4791fc8a795c2970f5c042f4ded664da39690 Author: Miklos Vajna <[email protected]> AuthorDate: Wed Jun 11 08:26:10 2025 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Thu Jun 12 08:20:53 2025 +0200 tdf#166319 sw interdependent redlines: fix undo of reject of ins-then-del's del The bugdoc has <ins>AA<del>BB</del>CC</ins> in it, rejecting BB + undo resulted in a plain delete redline for BB instead of a delete with an underlying insert. What happens is that the SwUndoRedline ctor only copied the "delete" redline data to the undo stack instaed of both "delete" and "insert", because copying the "next redline data" (i.e. insert) was decided based on the undo action type, so didn't happen for SwUndoId::REJECT_REDLINE, since commit b229d16cd94d31eb8cad043b54d2d421cf92ef70 (Related: i#123480 correct the previous made refactoring, 2014-03-04), which is a bit unclear, but the point is don't copy all redline data unconditionally. Fix the problem by extending SwUndoRedline with an explicit flag, so sw::DocumentRedlineManager::RejectRedlineRange() can opt in to request a copy of both redline data objects. This also simplifies RejectRedlineRange(), because now it can again create exactly one SwUndoRejectRedline instance on the undo stack. This also allows removing the previous workaround where a reject action created an SwUndoAcceptRedline to force the copy of both redline data instances. Change-Id: Ieb805b52c05b0dd4eb556c720fb941f3bb394bd4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186368 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins (cherry picked from commit 612ba7b2bc5d1d12c10287094263f6d31983a3d8) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186377 diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx index 4efd50044228..dee077dcd7a9 100644 --- a/sw/qa/core/doc/doc.cxx +++ b/sw/qa/core/doc/doc.cxx @@ -738,6 +738,42 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testInsThenDelRejectUndo) // i.e. initially the doc had no redlines after insert, but undo + doing it again resulted in // redlines, which is inconsistent. CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), rRedlines.size()); + + // And given a reset state, matching the one after import: + pWrtShell->Undo(); + { + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlines[0]->GetType()); + const SwRedlineData& rRedlineData1 = rRedlines[1]->GetRedlineData(0); + CPPUNIT_ASSERT_EQUAL(RedlineType::Delete, rRedlineData1.GetType()); + CPPUNIT_ASSERT(rRedlineData1.Next()); + const SwRedlineData& rInnerRedlineData = *rRedlineData1.Next(); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rInnerRedlineData.GetType()); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlines[2]->GetType()); + } + + // When rejecting the insert-then-delete + undo: + SwCursor* pCursor = pWrtShell->GetCursor(); + pCursor->DeleteMark(); + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false); + SwRedlineTable::size_type nRedline{}; + rRedlines.FindAtPosition(*pCursor->Start(), nRedline); + // A redline is found. + CPPUNIT_ASSERT_LESS(rRedlines.size(), nRedline); + pWrtShell->RejectRedline(nRedline); + pWrtShell->Undo(); + + // Then make sure that the restored redline has 2 redlines datas: delete and insert: + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlines[0]->GetType()); + const SwRedlineData& rRedlineData1 = rRedlines[1]->GetRedlineData(0); + CPPUNIT_ASSERT_EQUAL(RedlineType::Delete, rRedlineData1.GetType()); + // The insert "under" the delete was lost. + CPPUNIT_ASSERT(rRedlineData1.Next()); + const SwRedlineData& rInnerRedlineData = *rRedlineData1.Next(); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rInnerRedlineData.GetType()); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlines[2]->GetType()); } CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testInsThenFormat) diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index 64d3b108aaa2..54127a714826 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -3648,21 +3648,11 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr } else if (pTmp->GetRedlineData(0).CanCombineForAcceptReject(aOrigData)) { - bool bHierarchicalFormat - = pTmp->GetType() == RedlineType::Format && pTmp->GetStackCount() > 1; + bool bHierarchical = pTmp->GetStackCount() > 1; + bool bHierarchicalFormat = bHierarchical && pTmp->GetType() == RedlineType::Format; if (m_rDoc.GetIDocumentUndoRedo().DoesUndo()) { - std::unique_ptr<SwUndoRedline> pUndoRdl; - if (bHierarchicalFormat) - { - // Format on another type: just create an accept undo action, we'll deal with - // insert or delete below separately. - pUndoRdl = std::make_unique<SwUndoAcceptRedline>(*pTmp); - } - else - { - pUndoRdl = std::make_unique<SwUndoRejectRedline>(*pTmp); - } + auto pUndoRdl = std::make_unique<SwUndoRejectRedline>(*pTmp, 0, bHierarchical); #if OSL_DEBUG_LEVEL > 0 pUndoRdl->SetRedlineCountDontCheck(true); #endif diff --git a/sw/source/core/inc/UndoRedline.hxx b/sw/source/core/inc/UndoRedline.hxx index 955841b47d92..1e1033d319d5 100644 --- a/sw/source/core/inc/UndoRedline.hxx +++ b/sw/source/core/inc/UndoRedline.hxx @@ -37,12 +37,14 @@ protected: SwUndoId mnUserId; bool mbHiddenRedlines; sal_Int8 mnDepth; // index of the SwRedlineData in SwRangeRedline + /// If the redline has multiple SwRedlineData instances associated that we want to track. + bool mbHierarchical; virtual void UndoRedlineImpl(SwDoc & rDoc, SwPaM & rPam); virtual void RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam); public: - SwUndoRedline( SwUndoId nUserId, const SwPaM& rRange, sal_Int8 nDepth = 0 ); + SwUndoRedline( SwUndoId nUserId, const SwPaM& rRange, sal_Int8 nDepth = 0, bool bHierarchical = false ); virtual ~SwUndoRedline() override; @@ -123,7 +125,7 @@ private: virtual void RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam) override; public: - SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth = 0 ); + SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth = 0, bool bHierarchical = false ); virtual void RepeatImpl( ::sw::RepeatContext & ) override; }; diff --git a/sw/source/core/undo/unredln.cxx b/sw/source/core/undo/unredln.cxx index 8456ef83098a..d4bbef46dc86 100644 --- a/sw/source/core/undo/unredln.cxx +++ b/sw/source/core/undo/unredln.cxx @@ -37,11 +37,12 @@ #include <sortopt.hxx> #include <docedt.hxx> -SwUndoRedline::SwUndoRedline( SwUndoId nUsrId, const SwPaM& rRange, sal_Int8 nDepth ) +SwUndoRedline::SwUndoRedline( SwUndoId nUsrId, const SwPaM& rRange, sal_Int8 nDepth, bool bHierarchical ) : SwUndo( SwUndoId::REDLINE, rRange.GetDoc() ), SwUndRng( rRange ), mnUserId( nUsrId ), mbHiddenRedlines( false ), - mnDepth( nDepth ) + mnDepth( nDepth ), + mbHierarchical(bHierarchical) { // consider Redline SwDoc& rDoc = rRange.GetDoc(); @@ -62,7 +63,12 @@ SwUndoRedline::SwUndoRedline( SwUndoId nUsrId, const SwPaM& rRange, sal_Int8 nDe SwNodeOffset nEndExtra = rDoc.GetNodes().GetEndOfExtras().GetIndex(); mpRedlSaveData.reset( new SwRedlineSaveDatas ); - if( !FillSaveData( rRange, *mpRedlSaveData, false, SwUndoId::REJECT_REDLINE != mnUserId )) + bool bCopyNext = SwUndoId::REJECT_REDLINE != mnUserId; + if (mbHierarchical) + { + bCopyNext = true; + } + if( !FillSaveData( rRange, *mpRedlSaveData, false, bCopyNext )) { mpRedlSaveData.reset(); } @@ -115,6 +121,11 @@ void SwUndoRedline::dumpAsXml(xmlTextWriterPtr pWriter) const mpRedlSaveData->dumpAsXml(pWriter); } + (void)xmlTextWriterStartElement(pWriter, BAD_CAST("hierarchical")); + (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("value"), + BAD_CAST(OString::boolean(mbHierarchical).getStr())); + (void)xmlTextWriterEndElement(pWriter); + (void)xmlTextWriterEndElement(pWriter); } @@ -462,8 +473,8 @@ void SwUndoAcceptRedline::RepeatImpl(::sw::RepeatContext & rContext) rContext.GetDoc().getIDocumentRedlineAccess().AcceptRedline(rContext.GetRepeatPaM(), true); } -SwUndoRejectRedline::SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */ ) - : SwUndoRedline( SwUndoId::REJECT_REDLINE, rRange, nDepth ) +SwUndoRejectRedline::SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */, bool bHierarchical /*= false*/ ) + : SwUndoRedline( SwUndoId::REJECT_REDLINE, rRange, nDepth, bHierarchical ) { }
