sw/inc/IDocumentRedlineAccess.hxx | 12 + sw/inc/editsh.hxx | 4 sw/qa/core/doc/DocumentRedlineManager.cxx | 181 ++++++++++++++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 106 ++++++++++++--- sw/source/core/edit/edredln.cxx | 8 - sw/source/core/inc/DocumentRedlineManager.hxx | 18 +- sw/source/core/inc/UndoRedline.hxx | 13 + sw/source/core/undo/unredln.cxx | 21 +-- sw/source/uibase/misc/redlndlg.cxx | 6 9 files changed, 317 insertions(+), 52 deletions(-)
New commits: commit 7afa99daab032723aabdeea00d6d69cf97928434 Author: Miklos Vajna <[email protected]> AuthorDate: Tue Oct 7 08:27:32 2025 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Oct 21 11:21:22 2025 +0200 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 <ins>AAA</ins>BBB<ins>CCC</ins> 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 <del>AAA</del><format>BBB</format><del>CCC</del> 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 <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192736 diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index 47bd80212825..14efdbff0bd8 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -209,7 +209,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 <del>AAA</del><format>BBB</format><del>CCC</del> instead of one big delete + // redline. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(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 <ins>AAA</ins>BBB<ins>CCC</ins> instead of one big insert. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(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<SwUndoRejectRedline>(*pTmp, nDepth, bHierarchical); + auto pUndoRdl = std::make_unique<SwUndoRejectRedline>(*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 3d3ec7028ede..053ea0ef97cc 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -113,7 +113,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 cbdd4562c65f..cc3466c0fc75 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) commit fbec8ab124b58779567e14428238136baa1e84d6 Author: Miklos Vajna <[email protected]> AuthorDate: Mon Oct 6 08:40:26 2025 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Oct 21 11:21:06 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/+/191960 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192735 diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index ab3eefe59a1a..47bd80212825 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -197,7 +197,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 bec423d64351..5f4d0b796023 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 dd5c52074313..00cb23fe6ffc 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -3393,15 +3393,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(); @@ -3597,7 +3598,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) != @@ -3625,7 +3626,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 0728861c4953..3d3ec7028ede 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -103,7 +103,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 ec355d1f3fd0..cbdd4562c65f 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) commit 772189ecaca99338672d94aeb1f34abb836b7f3e Author: Miklos Vajna <[email protected]> AuthorDate: Mon Sep 29 08:44:22 2025 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Oct 21 11:20:44 2025 +0200 tdf#166319 sw interdependent redlines: allow accept/reject for fmt on ins/del When a format redline is on top of an insert/delete and the uno command is dispatched to accept/reject the redline under cursor, then we always interact with the insert/delete, ignoring the format. This is usually wanted, but now there is no way to interact with the format redline, even if you would explicitly ask for it. Notice that the manage changes dialog/sidebar has a separate row for the format redline and the ins/del redline under it (at the same document model position), so that gives us a way to select the format redline explicitly and interact with it. Implement this by adding a "direct" mode to the edit shell's AcceptRedline() / RejectRedline(). With this, all of the (insert or delete) then format, (accept or reject) on it case works in direct mode. Undo also works, redo still needs fixing. Change-Id: I7947cd6c7264773d51e63ccd7c45acb001a79d88 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191592 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192734 diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index 7861a5858421..ab3eefe59a1a 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -192,7 +192,8 @@ public: virtual void SetRedlineMove(/*[in]*/bool bFlag) = 0; virtual bool AcceptRedline(/*[in]*/ SwRedlineTable::size_type nPos, /*[in]*/ bool bCallDelete, - /*[in]*/ bool bRange = false) + /*[in]*/ bool bRange = false, + bool bDirect = false) = 0; virtual bool AcceptRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, @@ -202,7 +203,8 @@ public: virtual void AcceptRedlineParagraphFormatting(/*[in]*/const SwPaM& rPam ) = 0; virtual bool RejectRedline(/*[in]*/ SwRedlineTable::size_type nPos, - /*[in]*/ bool bCallDelete, /*[in]*/ bool bRange = false) + /*[in]*/ bool bCallDelete, /*[in]*/ bool bRange = false, + bool bDirect = false) = 0; virtual bool RejectRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, diff --git a/sw/inc/editsh.hxx b/sw/inc/editsh.hxx index 67506453b8f3..656c9e6b3a42 100644 --- a/sw/inc/editsh.hxx +++ b/sw/inc/editsh.hxx @@ -959,8 +959,8 @@ public: SW_DLLPUBLIC SwRedlineTable::size_type GetRedlineCount() const; const SwRangeRedline& GetRedline( SwRedlineTable::size_type nPos ) const; SwRangeRedline& GetRedline( SwRedlineTable::size_type nPos ); - SW_DLLPUBLIC bool AcceptRedline( SwRedlineTable::size_type nPos ); - SW_DLLPUBLIC bool RejectRedline( SwRedlineTable::size_type nPos ); + SW_DLLPUBLIC bool AcceptRedline( SwRedlineTable::size_type nPos, bool bDirect = false ); + SW_DLLPUBLIC bool RejectRedline( SwRedlineTable::size_type nPos, bool bDirect = false ); bool AcceptRedlinesInSelection(); bool RejectRedlinesInSelection(); SW_DLLPUBLIC void ReinstateRedline(SwRedlineTable::size_type nPos); diff --git a/sw/qa/core/doc/DocumentRedlineManager.cxx b/sw/qa/core/doc/DocumentRedlineManager.cxx index 10221f79664a..bec423d64351 100644 --- a/sw/qa/core/doc/DocumentRedlineManager.cxx +++ b/sw/qa/core/doc/DocumentRedlineManager.cxx @@ -21,6 +21,7 @@ #include <swmodule.hxx> #include <strings.hrc> #include <fchrfmt.hxx> +#include <ndtxt.hxx> namespace { @@ -172,6 +173,131 @@ CPPUNIT_TEST_FIXTURE(Test, testFormatRedlineRecordOldCharStyle) const SwFormatCharFormat& rOldCharFormat = pRedlineSet->Get(RES_TXTATR_CHARFMT); CPPUNIT_ASSERT_EQUAL(u"Emphasis"_ustr, rOldCharFormat.GetCharFormat()->GetName().toString()); } + +CPPUNIT_TEST_FIXTURE(Test, testDelThenFormatDirect) +{ + // Given a document with a delete redline, part of it has a format redline on top: + createSwDoc("del-then-format.docx"); + + // When "directly" accepting the delete-then-format redline: + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + pWrtShell->AcceptRedline(1, /*bDirect=*/true); + + // Then make sure that the format gets accepted, and the delete redline is kept: + SwDoc* pDoc = pDocShell->GetDoc(); + IDocumentRedlineAccess& rIDRA = pDoc->getIDocumentRedlineAccess(); + SwRedlineTable& rRedlines = rIDRA.GetRedlineTable(); + { + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 0 + // i.e. when ignoring the "direct" parameter, the delete redline was accepted instead of the + // format one. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + const SwRedlineData& rRedlineData = rRedlines[0]->GetRedlineData(0); + CPPUNIT_ASSERT_EQUAL(RedlineType::Delete, rRedlineData.GetType()); + CPPUNIT_ASSERT(!rRedlineData.Next()); + + // After first char inside the redline: bold. + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false); + SfxItemSetFixed<RES_CHRATR_WEIGHT, RES_CHRATR_WEIGHT> aSet(pDoc->GetAttrPool()); + pWrtShell->GetCurAttr(aSet); + const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); + CPPUNIT_ASSERT_EQUAL(WEIGHT_BOLD, rWeightItem.GetValue()); + } + + // And given a reset state: + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + + // When "directly" rejecting the delete-then-format redline: + pWrtShell->RejectRedline(1, /*bDirect=*/true); + + // Then make sure that the format gets rejected and the delete redline is kept: + { + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + const SwRedlineData& rRedlineData = rRedlines[0]->GetRedlineData(0); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 (delete) + // - Actual : 2 (format) + // i.e. the delete redline was rejected and format remained, not the other way around. + CPPUNIT_ASSERT_EQUAL(RedlineType::Delete, rRedlineData.GetType()); + CPPUNIT_ASSERT(!rRedlineData.Next()); + + // After first char inside the redline: not bold anymore. + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false); + SfxItemSetFixed<RES_CHRATR_WEIGHT, RES_CHRATR_WEIGHT> aSet(pDoc->GetAttrPool()); + pWrtShell->GetCurAttr(aSet); + const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); + CPPUNIT_ASSERT_EQUAL(WEIGHT_NORMAL, rWeightItem.GetValue()); + } +} + +CPPUNIT_TEST_FIXTURE(Test, testInsThenFormatDirect) +{ + // Given a document with an insert redline, part of it has a format redline on top: + createSwDoc("ins-then-format.docx"); + + // When "directly" accepting the insert-then-format redline: + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + pWrtShell->AcceptRedline(1, /*bDirect=*/true); + + // Then make sure that the format gets accepted, and the insert redline is kept: + SwDoc* pDoc = pDocShell->GetDoc(); + IDocumentRedlineAccess& rIDRA = pDoc->getIDocumentRedlineAccess(); + SwRedlineTable& rRedlines = rIDRA.GetRedlineTable(); + { + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 3 + // i.e. the accept didn't do anything in direct mode. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + const SwRedlineData& rRedlineData = rRedlines[0]->GetRedlineData(0); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlineData.GetType()); + CPPUNIT_ASSERT(!rRedlineData.Next()); + + // After first char inside the redline: bold. + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false); + SfxItemSetFixed<RES_CHRATR_WEIGHT, RES_CHRATR_WEIGHT> aSet(pDoc->GetAttrPool()); + pWrtShell->GetCurAttr(aSet); + const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); + CPPUNIT_ASSERT_EQUAL(WEIGHT_BOLD, rWeightItem.GetValue()); + } + + // And given a reset state: + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rRedlines.size()); + + // When "directly" rejecting the insert-then-format redline: + pWrtShell->RejectRedline(1, /*bDirect=*/true); + + // Then make sure that the format gets rejected and the insert redline is kept: + { + SwTextNode* pTextNode = pWrtShell->GetCursor()->GetPointNode().GetTextNode(); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: AAABBBCCC + // - Actual : AAACCC + // i.e. the insert was rejected, not the format. + CPPUNIT_ASSERT_EQUAL(u"AAABBBCCC"_ustr, pTextNode->GetText()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + const SwRedlineData& rRedlineData = rRedlines[0]->GetRedlineData(0); + CPPUNIT_ASSERT_EQUAL(RedlineType::Insert, rRedlineData.GetType()); + CPPUNIT_ASSERT(!rRedlineData.Next()); + + // After first char inside the redline: not bold anymore. + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false); + SfxItemSetFixed<RES_CHRATR_WEIGHT, RES_CHRATR_WEIGHT> aSet(pDoc->GetAttrPool()); + pWrtShell->GetCurAttr(aSet); + const SvxWeightItem& rWeightItem = aSet.Get(RES_CHRATR_WEIGHT); + CPPUNIT_ASSERT_EQUAL(WEIGHT_NORMAL, rWeightItem.GetValue()); + } +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index d05a4b38b013..dd5c52074313 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -1016,6 +1016,27 @@ namespace return true; } + /// Given a redline that has an other underlying redline, drop the redline on top. + /// Used to accept a format on top of insert/delete, no changes to the text node string. + bool lcl_AcceptOuterFormat(SwRedlineTable& rArr, SwRedlineTable::size_type& rPos) + { + SwRangeRedline* pRedl = rArr[rPos]; + return pRedl->PopData(); + } + + /// Given a redline that has an other underlying redline, drop the redline on top & restore the + /// old doc model. Used to reject a format on top of insert/delete. + bool lcl_RejectOuterFormat(SwRedlineTable& rArr, SwRedlineTable::size_type& rPos) + { + SwRangeRedline* pRedl = rArr[rPos]; + SwDoc& rDoc = pRedl->GetDoc(); + SwPaM aPam(*(pRedl->Start()), *(pRedl->End())); + rDoc.ResetAttrs(aPam); + if (pRedl->GetExtraData()) + pRedl->GetExtraData()->Reject(*pRedl); + return pRedl->PopData(); + } + /// Given a redline that has two types and the underlying type is /// delete, reject the redline based on that underlying type. Used /// to accept a delete-then-format, i.e. this does change the text @@ -3341,7 +3362,7 @@ const SwRangeRedline* DocumentRedlineManager::GetRedline( const SwPosition& rPos bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOrigin, const SwRedlineTable::size_type& nPosStart, const SwRedlineTable::size_type& nPosEnd, - bool bCallDelete) + bool bCallDelete, bool bDirect) { bool bRet = false; @@ -3385,7 +3406,13 @@ bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOr nPamEndNI = pTmp->Start()->GetNodeIndex(); nPamEndCI = pTmp->Start()->GetContentIndex(); - if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert) + if (bHierarchicalFormat && bDirect + && (pTmp->GetType(1) == RedlineType::Insert + || pTmp->GetType(1) == RedlineType::Delete)) + { + bRet |= lcl_AcceptOuterFormat(maRedlineTable, nRdlIdx); + } + else if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert) { // This combination of 2 redline types prefers accepting the inner one first. bRet |= lcl_DeleteInnerRedline(maRedlineTable, nRdlIdx, 1); @@ -3425,7 +3452,7 @@ bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOr } nRdlIdx++; //we will decrease it in the loop anyway. } - else if (CanReverseCombineTypesForAcceptReject(*pTmp, aOrigData)) + else if (CanReverseCombineTypesForAcceptReject(*pTmp, aOrigData) && !bDirect) { // The aOrigData has 2 types and for these types we want the underlying type to be // combined with the type of the surrounding redlines, so accept pTmp, too. @@ -3474,7 +3501,7 @@ bool DocumentRedlineManager::AcceptMovedRedlines(sal_uInt32 nMovedID, bool bCall } bool DocumentRedlineManager::AcceptRedline(SwRedlineTable::size_type nPos, bool bCallDelete, - bool bRange) + bool bRange, bool bDirect) { bool bRet = false; @@ -3519,7 +3546,7 @@ bool DocumentRedlineManager::AcceptRedline(SwRedlineTable::size_type nPos, bool // Accept redlines between pPamStart-pPamEnd. // but only those that can be combined with the selected. - bRet |= AcceptRedlineRange(nPos, nPosStart, nPosEnd, bCallDelete); + bRet |= AcceptRedlineRange(nPos, nPosStart, nPosEnd, bCallDelete, bDirect); } } else do { @@ -3660,7 +3687,7 @@ void DocumentRedlineManager::AcceptRedlineParagraphFormatting( const SwPaM &rPam bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOrigin, const SwRedlineTable::size_type& nPosStart, const SwRedlineTable::size_type& nPosEnd, - bool bCallDelete) + bool bCallDelete, bool bDirect) { bool bRet = false; @@ -3708,7 +3735,13 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr nPamEndNI = pTmp->Start()->GetNodeIndex(); nPamEndCI = pTmp->Start()->GetContentIndex(); - if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert) + if (bHierarchicalFormat && bDirect + && (pTmp->GetType(1) == RedlineType::Insert + || pTmp->GetType(1) == RedlineType::Delete)) + { + bRet |= lcl_RejectOuterFormat(maRedlineTable, nRdlIdx); + } + else if (bHierarchicalFormat && pTmp->GetType(1) == RedlineType::Insert) { // Accept the format itself and then reject the insert by deleting the range. SwPaM aPam(*pTmp->Start(), *pTmp->End()); @@ -3778,7 +3811,7 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr } nRdlIdx++; //we will decrease it in the loop anyway. } - else if (CanReverseCombineTypesForAcceptReject(*pTmp, aOrigData)) + else if (CanReverseCombineTypesForAcceptReject(*pTmp, aOrigData) && !bDirect) { // The aOrigData has 2 types and for these types we want the underlying type to be // combined with the type of the surrounding redlines, so reject pTmp, too. @@ -3833,7 +3866,8 @@ bool DocumentRedlineManager::RejectMovedRedlines(sal_uInt32 nMovedID, bool bCall } bool DocumentRedlineManager::RejectRedline(SwRedlineTable::size_type nPos, - bool bCallDelete, bool bRange) + bool bCallDelete, bool bRange, + bool bDirect) { bool bRet = false; @@ -3878,7 +3912,7 @@ bool DocumentRedlineManager::RejectRedline(SwRedlineTable::size_type nPos, // Reject items between pPamStart-pPamEnd // but only those that can be combined with the selected. - bRet |= RejectRedlineRange(nPos, nPosStart, nPosEnd, bCallDelete); + bRet |= RejectRedlineRange(nPos, nPosStart, nPosEnd, bCallDelete, bDirect); } } else do { diff --git a/sw/source/core/edit/edredln.cxx b/sw/source/core/edit/edredln.cxx index 74d1a8d3def1..d9b1961db998 100644 --- a/sw/source/core/edit/edredln.cxx +++ b/sw/source/core/edit/edredln.cxx @@ -83,11 +83,11 @@ static void lcl_InvalidateAll( SwViewShell* pSh ) } } -bool SwEditShell::AcceptRedline( SwRedlineTable::size_type nPos ) +bool SwEditShell::AcceptRedline( SwRedlineTable::size_type nPos, bool bDirect ) { CurrShell aCurr( this ); StartAllAction(); - bool bRet = GetDoc()->getIDocumentRedlineAccess().AcceptRedline( nPos, true, true ); + bool bRet = GetDoc()->getIDocumentRedlineAccess().AcceptRedline( nPos, true, true, bDirect ); if( !nPos && !::IsExtraData( *GetDoc() ) ) lcl_InvalidateAll( this ); EndAllAction(); @@ -170,11 +170,11 @@ void SwEditShell::ReinstateRedline(SwRedlineTable::size_type nPos) EndAllAction(); } -bool SwEditShell::RejectRedline( SwRedlineTable::size_type nPos ) +bool SwEditShell::RejectRedline( SwRedlineTable::size_type nPos, bool bDirect ) { CurrShell aCurr( this ); StartAllAction(); - bool bRet = GetDoc()->getIDocumentRedlineAccess().RejectRedline( nPos, true, true ); + bool bRet = GetDoc()->getIDocumentRedlineAccess().RejectRedline( nPos, true, true, bDirect ); if( !nPos && !::IsExtraData( *GetDoc() ) ) lcl_InvalidateAll( this ); EndAllAction(); diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx b/sw/source/core/inc/DocumentRedlineManager.hxx index bc4187406dc0..0728861c4953 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -99,7 +99,8 @@ public: virtual void SetRedlineMove(/*[in]*/bool bFlag) override; virtual bool AcceptRedline(/*[in]*/ SwRedlineTable::size_type nPos, /*[in]*/ bool bCallDelete, - /*[in]*/ bool bRange = false) override; + /*[in]*/ bool bRange = false, + bool bDirect = false) override; virtual bool AcceptRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, /*[in]*/ sal_Int8 nDepth = 0) override; @@ -107,7 +108,8 @@ public: virtual void AcceptRedlineParagraphFormatting(/*[in]*/const SwPaM& rPam) override; virtual bool RejectRedline(/*[in]*/ SwRedlineTable::size_type nPos, /*[in]*/ bool bCallDelete, - /*[in]*/ bool bRange = false) override; + /*[in]*/ bool bRange = false, + bool bDirect = false) override; virtual bool RejectRedline(/*[in]*/ const SwPaM& rPam, /*[in]*/ bool bCallDelete, /*[in]*/ sal_Int8 nDepth = 0) override; @@ -159,10 +161,12 @@ private: bool RejectRedlineRange(SwRedlineTable::size_type nPosOrigin, const SwRedlineTable::size_type& nPosStart, - const SwRedlineTable::size_type& nPosEnd, bool bCallDelete); + const SwRedlineTable::size_type& nPosEnd, bool bCallDelete, + bool bDirect); bool AcceptRedlineRange(SwRedlineTable::size_type nPosOrigin, const SwRedlineTable::size_type& nPosStart, - const SwRedlineTable::size_type& nPosEnd, bool bCallDelete); + const SwRedlineTable::size_type& nPosEnd, bool bCallDelete, + bool bDirect); bool AcceptMovedRedlines(sal_uInt32 nMovedID, bool bCallDelete); bool RejectMovedRedlines(sal_uInt32 nMovedID, bool bCallDelete); void PreAppendInsertRedline(AppendRedlineContext& rCtx); diff --git a/sw/source/uibase/misc/redlndlg.cxx b/sw/source/uibase/misc/redlndlg.cxx index d43e7ac0af98..49fa45217c43 100644 --- a/sw/source/uibase/misc/redlndlg.cxx +++ b/sw/source/uibase/misc/redlndlg.cxx @@ -1138,7 +1138,7 @@ void SwRedlineAcceptDlg::CallAcceptReject( bool bSelect, bool bAccept ) else rTreeView.all_foreach(lambda); - bool (SwEditShell::*FnAccRej)( SwRedlineTable::size_type ) = &SwEditShell::AcceptRedline; + bool (SwEditShell::*FnAccRej)( SwRedlineTable::size_type, bool ) = &SwEditShell::AcceptRedline; if( !bAccept ) FnAccRej = &SwEditShell::RejectRedline; @@ -1185,7 +1185,7 @@ void SwRedlineAcceptDlg::CallAcceptReject( bool bSelect, bool bAccept ) { SwRedlineTable::size_type nPosition = GetRedlinePos( *rRedLine ); if( nPosition != SwRedlineTable::npos ) - (pSh->*FnAccRej)( nPosition ); + (pSh->*FnAccRej)( nPosition, /*bDirect=*/true ); // handle redlines of table rows, stored as children of the item associated // to the deleted/inserted table row(s) @@ -1200,7 +1200,7 @@ void SwRedlineAcceptDlg::CallAcceptReject( bool bSelect, bool bAccept ) { nPosition = GetRedlinePos( *xChild ); if( nPosition != SwRedlineTable::npos ) - (pSh->*FnAccRej)( nPosition ); + (pSh->*FnAccRej)( nPosition, /*bDirect=*/true ); } while ( rTreeView.iter_next_sibling(*xChild) ); }
