From 1e3b8177aef8cc3a242cd9fd41d492f2fd89d21e Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 7 Oct 2025 08:27:32 +0200 Subject: [PATCH] tdf#166319 sw interdependent redlines: fix redo of direct reject for format Open the bugdoc, select the format part of the insert-then-format redline in the manage changes dialog, reject, undo, redo: the result is AAABBBCCC instead of one big insert redline. Similarly, open the other bugdoc with a delete and a format redline in it, select the format part of the delete-then-format redline in the manage changes dialog, reject, undo, redo: the result is AAABBBCCC instead of one big delete redline. What happens is that the UI action uses sw::DocumentRedlineManager::RejectRedlineRange(), while the redo uses sw::DocumentRedlineManager::RejectRedline(), and while the first supported direct accepts, the later did not. Fix the problem by: 1) In sw::DocumentRedlineManager::RejectRedlineRange(), create the undo action with the correct depth & directness for direct rejects 2) In DocumentRedlineManager::RejectRedline(), use use lcl_RejectOuterFormat() to directly reject a format redline instead of the usual lcl_AcceptRejectRedl(), which gives us matching behavior for the UI action & the redo. Writer's interdependent redlines are now in a reasonable state with this: actions for the redline under cursor work with ins/del when possible (looking "through" format); while the manage changes dialog still allows working on the format redline directly if that's wanted. Change-Id: Idb90dfbd86ac26ff51da8a34731ea75a48748bdf Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192015 Tested-by: Jenkins Reviewed-by: Miklos Vajna --- sw/inc/IDocumentRedlineAccess.hxx | 3 ++- sw/qa/core/doc/DocumentRedlineManager.cxx | 27 +++++++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 25 ++++++++++++----- sw/source/core/inc/DocumentRedlineManager.hxx | 3 ++- sw/source/core/inc/UndoRedline.hxx | 2 +- sw/source/core/undo/unredln.cxx | 6 ++--- 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index dd37a92c9fc7..da6906b06bec 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -217,7 +217,8 @@ public: = 0; virtual bool RejectRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, - /*[in]*/ sal_Int8 nDepth = 0) + /*[in]*/ sal_Int8 nDepth = 0, + bool bDirect = false) = 0; virtual const SwRangeRedline* SelNextRedline(/*[in]*/SwPaM& rPam) const = 0; diff --git a/sw/qa/core/doc/DocumentRedlineManager.cxx b/sw/qa/core/doc/DocumentRedlineManager.cxx index 5f4d0b796023..15c2b432bb08 100644 --- a/sw/qa/core/doc/DocumentRedlineManager.cxx +++ b/sw/qa/core/doc/DocumentRedlineManager.cxx @@ -248,6 +248,20 @@ CPPUNIT_TEST_FIXTURE(Test, testDelThenFormatDirect) const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); CPPUNIT_ASSERT_EQUAL(WEIGHT_NORMAL, rWeightItem.GetValue()); } + + // And given an undo: + pWrtShell->Undo(); + + // When redoing: + pWrtShell->Redo(); + + // Then make sure that we only get a single big delete redline: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 3 + // i.e. we got AAABBBCCC instead of one big delete + // redline. + CPPUNIT_ASSERT_EQUAL(static_cast(1), rRedlines.size()); } CPPUNIT_TEST_FIXTURE(Test, testInsThenFormatDirect) @@ -325,6 +339,19 @@ CPPUNIT_TEST_FIXTURE(Test, testInsThenFormatDirect) const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); CPPUNIT_ASSERT_EQUAL(WEIGHT_NORMAL, rWeightItem.GetValue()); } + + // And given an undo: + pWrtShell->Undo(); + + // When redoing: + pWrtShell->Redo(); + + // Then make sure that we only get a single big insert redline: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 2 + // i.e. we got AAABBBCCC instead of one big insert. + CPPUNIT_ASSERT_EQUAL(static_cast(1), rRedlines.size()); } } diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index 00cb23fe6ffc..8be6b5b76515 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -3739,13 +3739,13 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr if (m_rDoc.GetIDocumentUndoRedo().DoesUndo()) { sal_Int8 nDepth = 0; - if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Delete) + if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Delete && !bDirect) { // Only work with the underlying delete, so the undo action matches the UI // action below. nDepth = 1; } - auto pUndoRdl = std::make_unique(*pTmp, nDepth, bHierarchical); + auto pUndoRdl = std::make_unique(*pTmp, nDepth, bHierarchical, bDirect); #if OSL_DEBUG_LEVEL > 0 pUndoRdl->SetRedlineCountDontCheck(true); #endif @@ -3982,7 +3982,7 @@ bool DocumentRedlineManager::RejectRedline(SwRedlineTable::size_type nPos, // #TODO - add 'SwExtraRedlineTable' also ? } -bool DocumentRedlineManager::RejectRedline( const SwPaM& rPam, bool bCallDelete, sal_Int8 nDepth ) +bool DocumentRedlineManager::RejectRedline( const SwPaM& rPam, bool bCallDelete, sal_Int8 nDepth, bool bDirect ) { // Switch to visible in any case if( (RedlineFlags::ShowInsert | RedlineFlags::ShowDelete) != @@ -4002,14 +4002,27 @@ bool DocumentRedlineManager::RejectRedline( const SwPaM& rPam, bool bCallDelete, } int nRet = 0; + SwRedlineTable::size_type nRdlIdx = 0; + const SwRangeRedline* pRedline = maRedlineTable.FindAtPosition(*rPam.Start(), nRdlIdx); if (nDepth == 0) { - nRet = lcl_AcceptRejectRedl(lcl_RejectRedline, maRedlineTable, bCallDelete, aPam); + bool bHierarchicalFormat = pRedline && pRedline->GetType() == RedlineType::Format && pRedline->GetStackCount() > 1; + if (bHierarchicalFormat && bDirect && (pRedline->GetType(1) == RedlineType::Insert || pRedline->GetType(1) == RedlineType::Delete)) + { + // Direct reject: work with the format redline on top. + if (lcl_RejectOuterFormat(maRedlineTable, nRdlIdx)) + { + nRet = 1; + } + } + else + { + // Non-direct reject: work with all redlines under aPam. + nRet = lcl_AcceptRejectRedl(lcl_RejectRedline, maRedlineTable, bCallDelete, aPam); + } } else { - SwRedlineTable::size_type nRdlIdx = 0; - const SwRangeRedline* pRedline = maRedlineTable.FindAtPosition(*rPam.Start(), nRdlIdx); if (nDepth == 1 && pRedline && pRedline->GetType(0) == RedlineType::Format && pRedline->GetType(1) == RedlineType::Delete) { diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx b/sw/source/core/inc/DocumentRedlineManager.hxx index b93bdf24a2b8..e377b6ce8d10 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -118,7 +118,8 @@ public: bool bDirect = false) override; virtual bool RejectRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, - /*[in]*/ sal_Int8 nDepth = 0) override; + /*[in]*/ sal_Int8 nDepth = 0, + bool bDirect = false) override; virtual void AcceptAllRedline(/*[in]*/bool bAcceptReject) override; diff --git a/sw/source/core/inc/UndoRedline.hxx b/sw/source/core/inc/UndoRedline.hxx index ead00d21e043..6476b3e0b41d 100644 --- a/sw/source/core/inc/UndoRedline.hxx +++ b/sw/source/core/inc/UndoRedline.hxx @@ -129,7 +129,7 @@ private: virtual void RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam) override; public: - SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth = 0, bool bHierarchical = false ); + SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth = 0, bool bHierarchical = false, bool bDirect = false ); virtual void RepeatImpl( ::sw::RepeatContext & ) override; }; diff --git a/sw/source/core/undo/unredln.cxx b/sw/source/core/undo/unredln.cxx index f9081c5d2fb4..5b95ca89fc22 100644 --- a/sw/source/core/undo/unredln.cxx +++ b/sw/source/core/undo/unredln.cxx @@ -482,14 +482,14 @@ void SwUndoAcceptRedline::RepeatImpl(::sw::RepeatContext & rContext) rContext.GetDoc().getIDocumentRedlineAccess().AcceptRedline(rContext.GetRepeatPaM(), true); } -SwUndoRejectRedline::SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */, bool bHierarchical /*= false*/ ) - : SwUndoRedline( SwUndoId::REJECT_REDLINE, rRange, nDepth, bHierarchical ) +SwUndoRejectRedline::SwUndoRejectRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */, bool bHierarchical /*= false*/, bool bDirect /*= false*/ ) + : SwUndoRedline( SwUndoId::REJECT_REDLINE, rRange, nDepth, bHierarchical, bDirect ) { } void SwUndoRejectRedline::RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam) { - rDoc.getIDocumentRedlineAccess().RejectRedline(rPam, false, mnDepth); + rDoc.getIDocumentRedlineAccess().RejectRedline(rPam, false, mnDepth, mbDirect); } void SwUndoRejectRedline::RepeatImpl(::sw::RepeatContext & rContext)