sw/inc/IDocumentRedlineAccess.hxx | 3 +- sw/qa/core/doc/DocumentRedlineManager.cxx | 28 ++++++++++++++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 27 +++++++++++++++++++++---- sw/source/core/inc/DocumentRedlineManager.hxx | 3 +- sw/source/core/inc/UndoRedline.hxx | 11 ++++++---- sw/source/core/undo/unredln.cxx | 15 +++++++++---- 6 files changed, 72 insertions(+), 15 deletions(-)
New commits: commit bb0b02d1770daf650ec4d57afe3ea01c7e4a5167 Author: Miklos Vajna <[email protected]> AuthorDate: Mon Oct 6 08:40:26 2025 +0200 Commit: Caolán McNamara <[email protected]> CommitDate: Mon Oct 6 10:24:38 2025 +0200 tdf#166319 sw interdependent redlines: fix redo of accept for fmt on ins/del The bugdoc has <ins>AA<format>BB</format>CC</ins> content, accepting the BB part "directly (via the sidebar or the manage changes dialog, so you accept the format, not the underlying insert), then undo, then redo resulted in <ins>AA</ins><format>BB</format><ins>CC</ins> instead of a single insert, as expected. What happens is that the UI action uses sw::DocumentRedlineManager::AcceptRedlineRange(), while the redo uses sw::DocumentRedlineManager::AcceptRedline(), and while the first supported direct accepts, the later did not. Fix the problem by: 1) Extending SwUndoRedline with a flag to know if the accept/reject to be performed by redo should be a direct action or not 2) Fixing sw::DocumentRedlineManager::AcceptRedlineRange() to create an undo action with the correct depth (should be 0, was 1) & save the direct flag into the undo action 3) In sw::DocumentRedlineManager::AcceptRedline(), use lcl_AcceptOuterFormat() to accept a format redline directly instead of the usual lcl_AcceptRejectRedl(). This also fixes the case where you have a format on top of a delete (instead of an insert). Change-Id: I78d7ae9eebf8525191a3d9b3a1731d80ad04a75b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191936 Reviewed-by: Caolán McNamara <[email protected]> Tested-by: Jenkins CollaboraOffice <[email protected]> diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index 92e8c3857210..dd37a92c9fc7 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -205,7 +205,8 @@ public: = 0; virtual bool AcceptRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, - /*[in]*/ sal_Int8 nDepth = 0) + /*[in]*/ sal_Int8 nDepth = 0, + bool bDirect = false) = 0; virtual void AcceptRedlineParagraphFormatting(/*[in]*/const SwPaM& rPam ) = 0; diff --git a/sw/qa/core/doc/DocumentRedlineManager.cxx b/sw/qa/core/doc/DocumentRedlineManager.cxx index 23d48e205f49..260287e72fa9 100644 --- a/sw/qa/core/doc/DocumentRedlineManager.cxx +++ b/sw/qa/core/doc/DocumentRedlineManager.cxx @@ -208,6 +208,20 @@ CPPUNIT_TEST_FIXTURE(Test, testDelThenFormatDirect) CPPUNIT_ASSERT_EQUAL(WEIGHT_BOLD, rWeightItem.GetValue()); } + // And given an undo, so we can redo: + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + + // When doing the same again via redo: + pWrtShell->Redo(); + + // Then make sure we again have a single large delete: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 2 + // i.e. instead of a large delete redline, we have 2 small ones for AAA and CCC only. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + // And given a reset state: pWrtShell->Undo(); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); @@ -269,6 +283,20 @@ CPPUNIT_TEST_FIXTURE(Test, testInsThenFormatDirect) CPPUNIT_ASSERT_EQUAL(WEIGHT_BOLD, rWeightItem.GetValue()); } + // And given an undo, so we can redo: + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + + // When doing the same again via redo: + pWrtShell->Redo(); + + // Then make sure we again have a single large insert: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 3 + // i.e. redo was broken, the middle part was a format redline instead of an insert redline. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + // And given a reset state: pWrtShell->Undo(); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index 71bb4b32d59f..64c3a712debc 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -3394,15 +3394,16 @@ bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOr bool bHierarchicalFormat = pTmp->GetType() == RedlineType::Format && pTmp->GetStackCount() > 1; if (m_rDoc.GetIDocumentUndoRedo().DoesUndo()) { + // Set up the undo action with the correct depth & directness. sal_Int8 nDepth = 0; - if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert) + if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert && !bDirect) { // Only work with the underlying insert, so the undo action matches the UI // action below. nDepth = 1; } m_rDoc.GetIDocumentUndoRedo().AppendUndo( - std::make_unique<SwUndoAcceptRedline>(*pTmp, nDepth)); + std::make_unique<SwUndoAcceptRedline>(*pTmp, nDepth, bDirect)); } nPamEndNI = pTmp->Start()->GetNodeIndex(); nPamEndCI = pTmp->Start()->GetContentIndex(); @@ -3598,7 +3599,7 @@ bool DocumentRedlineManager::AcceptRedline(SwRedlineTable::size_type nPos, bool // #TODO - add 'SwExtraRedlineTable' also ? } -bool DocumentRedlineManager::AcceptRedline( const SwPaM& rPam, bool bCallDelete, sal_Int8 nDepth ) +bool DocumentRedlineManager::AcceptRedline( const SwPaM& rPam, bool bCallDelete, sal_Int8 nDepth, bool bDirect ) { // Switch to visible in any case if( (RedlineFlags::ShowInsert | RedlineFlags::ShowDelete) != @@ -3626,7 +3627,25 @@ bool DocumentRedlineManager::AcceptRedline( const SwPaM& rPam, bool bCallDelete, int nRet = 0; if (nDepth == 0) { - nRet = lcl_AcceptRejectRedl(lcl_AcceptRedline, maRedlineTable, bCallDelete, *pPam); + SwRedlineTable::size_type nRdlIdx = 0; + const SwRangeRedline* pRedline = maRedlineTable.FindAtPosition(*rPam.Start(), nRdlIdx); + bool bHierarchicalFormat = pRedline && pRedline->GetType() == RedlineType::Format + && pRedline->GetStackCount() > 1; + if (bHierarchicalFormat && bDirect + && (pRedline->GetType(1) == RedlineType::Insert + || pRedline->GetType(1) == RedlineType::Delete)) + { + // Direct accept: work with the format redline on top. + if (lcl_AcceptOuterFormat(maRedlineTable, nRdlIdx)) + { + nRet = 1; + } + } + else + { + // Non-direct accept: work with all redlines under pPam. + nRet = lcl_AcceptRejectRedl(lcl_AcceptRedline, maRedlineTable, bCallDelete, *pPam); + } } else { diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx b/sw/source/core/inc/DocumentRedlineManager.hxx index 35f05ed76f8a..47a1d2443a5a 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -108,7 +108,8 @@ public: bool bDirect = false) override; virtual bool AcceptRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, - /*[in]*/ sal_Int8 nDepth = 0) override; + /*[in]*/ sal_Int8 nDepth = 0, + bool bDirect = false) override; virtual void AcceptRedlineParagraphFormatting(/*[in]*/const SwPaM& rPam) override; diff --git a/sw/source/core/inc/UndoRedline.hxx b/sw/source/core/inc/UndoRedline.hxx index 4a7a707c8516..ead00d21e043 100644 --- a/sw/source/core/inc/UndoRedline.hxx +++ b/sw/source/core/inc/UndoRedline.hxx @@ -36,15 +36,18 @@ protected: std::unique_ptr<SwRedlineSaveDatas> mpRedlSaveData; 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. + /// Start at this index when processing the list of SwRedlineDatas. + sal_Int8 mnDepth; + /// When creating the undo action, copy "next" SwRedlineDatas or not? bool mbHierarchical; + /// Just process this item, not later ones when processing the list of SwRedlineDatas. + bool mbDirect; 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, bool bHierarchical = false ); + SwUndoRedline( SwUndoId nUserId, const SwPaM& rRange, sal_Int8 nDepth = 0, bool bHierarchical = false, bool bDirect = false ); virtual ~SwUndoRedline() override; @@ -114,7 +117,7 @@ private: virtual void RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam) override; public: - SwUndoAcceptRedline( const SwPaM& rRange, sal_Int8 nDepth = 0 ); + SwUndoAcceptRedline( const SwPaM& rRange, sal_Int8 nDepth = 0, 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 77c15f282b7d..6cacd98ee28b 100644 --- a/sw/source/core/undo/unredln.cxx +++ b/sw/source/core/undo/unredln.cxx @@ -37,12 +37,13 @@ #include <sortopt.hxx> #include <docedt.hxx> -SwUndoRedline::SwUndoRedline( SwUndoId nUsrId, const SwPaM& rRange, sal_Int8 nDepth, bool bHierarchical ) +SwUndoRedline::SwUndoRedline( SwUndoId nUsrId, const SwPaM& rRange, sal_Int8 nDepth, bool bHierarchical, bool bDirect ) : SwUndo( SwUndoId::REDLINE, &rRange.GetDoc() ), SwUndRng( rRange ), mnUserId( nUsrId ), mbHiddenRedlines( false ), mnDepth( nDepth ), - mbHierarchical(bHierarchical) + mbHierarchical(bHierarchical), + mbDirect(bDirect) { // consider Redline SwDoc& rDoc = rRange.GetDoc(); @@ -129,6 +130,10 @@ void SwUndoRedline::dumpAsXml(xmlTextWriterPtr pWriter) const (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("value"), BAD_CAST(OString::number(mnDepth).getStr())); (void)xmlTextWriterEndElement(pWriter); + (void)xmlTextWriterStartElement(pWriter, BAD_CAST("direct")); + (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("value"), + BAD_CAST(OString::boolean(mbDirect).getStr())); + (void)xmlTextWriterEndElement(pWriter); (void)xmlTextWriterEndElement(pWriter); } @@ -462,14 +467,14 @@ void SwUndoRedlineSort::SetSaveRange( const SwPaM& rRange ) m_nSaveEndContent = rPos.GetContentIndex(); } -SwUndoAcceptRedline::SwUndoAcceptRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */ ) - : SwUndoRedline( SwUndoId::ACCEPT_REDLINE, rRange, nDepth ) +SwUndoAcceptRedline::SwUndoAcceptRedline( const SwPaM& rRange, sal_Int8 nDepth /* = 0 */, bool bDirect ) + : SwUndoRedline( SwUndoId::ACCEPT_REDLINE, rRange, nDepth, /*bHierarhical=*/false, bDirect ) { } void SwUndoAcceptRedline::RedoRedlineImpl(SwDoc & rDoc, SwPaM & rPam) { - rDoc.getIDocumentRedlineAccess().AcceptRedline(rPam, false, mnDepth); + rDoc.getIDocumentRedlineAccess().AcceptRedline(rPam, false, mnDepth, mbDirect); } void SwUndoAcceptRedline::RepeatImpl(::sw::RepeatContext & rContext)
