sw/inc/IDocumentRedlineAccess.hxx | 6 - sw/inc/editsh.hxx | 4 sw/qa/core/doc/DocumentRedlineManager.cxx | 126 ++++++++++++++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 54 +++++++++-- sw/source/core/edit/edredln.cxx | 8 - sw/source/core/inc/DocumentRedlineManager.hxx | 12 +- sw/source/uibase/misc/redlndlg.cxx | 6 - 7 files changed, 191 insertions(+), 25 deletions(-)
New commits: commit 21dfd6c046930c5d5f189a64230d514469ee1f4f Author: Miklos Vajna <[email protected]> AuthorDate: Mon Sep 29 08:44:22 2025 +0200 Commit: Caolán McNamara <[email protected]> CommitDate: Mon Sep 29 09:57:05 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/+/191588 Tested-by: Caolán McNamara <[email protected]> Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx index e270ad92c57e..92e8c3857210 100644 --- a/sw/inc/IDocumentRedlineAccess.hxx +++ b/sw/inc/IDocumentRedlineAccess.hxx @@ -200,7 +200,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, @@ -210,7 +211,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 0e42dca687ee..814221520067 100644 --- a/sw/inc/editsh.hxx +++ b/sw/inc/editsh.hxx @@ -963,8 +963,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 7bd923ec34a2..23d48e205f49 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()); } + +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 d252c49797e4..71bb4b32d59f 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -1017,6 +1017,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 @@ -3342,7 +3363,7 @@ const SwRangeRedline* DocumentRedlineManager::GetRedline( const SwPosition& rPos bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOrigin, SwRedlineTable::size_type& nPosStart, SwRedlineTable::size_type& nPosEnd, - bool bCallDelete) + bool bCallDelete, bool bDirect) { bool bRet = false; @@ -3386,7 +3407,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); @@ -3426,7 +3453,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. @@ -3475,7 +3502,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; @@ -3520,7 +3547,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 { @@ -3661,7 +3688,7 @@ void DocumentRedlineManager::AcceptRedlineParagraphFormatting( const SwPaM &rPam bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOrigin, SwRedlineTable::size_type& nPosStart, SwRedlineTable::size_type& nPosEnd, - bool bCallDelete) + bool bCallDelete, bool bDirect) { bool bRet = false; @@ -3709,7 +3736,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()); @@ -3779,7 +3812,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. @@ -3834,7 +3867,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; @@ -3879,7 +3913,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 46838487f699..71364e83095d 100644 --- a/sw/source/core/edit/edredln.cxx +++ b/sw/source/core/edit/edredln.cxx @@ -81,11 +81,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(); @@ -168,11 +168,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 ad443f95ea34..35f05ed76f8a 100644 --- a/sw/source/core/inc/DocumentRedlineManager.hxx +++ b/sw/source/core/inc/DocumentRedlineManager.hxx @@ -104,7 +104,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; @@ -112,7 +113,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; @@ -164,10 +166,12 @@ private: bool RejectRedlineRange(SwRedlineTable::size_type nPosOrigin, SwRedlineTable::size_type& nPosStart, - SwRedlineTable::size_type& nPosEnd, bool bCallDelete); + SwRedlineTable::size_type& nPosEnd, bool bCallDelete, + bool bDirect); bool AcceptRedlineRange(SwRedlineTable::size_type nPosOrigin, SwRedlineTable::size_type& nPosStart, - SwRedlineTable::size_type& nPosEnd, bool bCallDelete); + 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 ae6ca0feb358..ac84002e0070 100644 --- a/sw/source/uibase/misc/redlndlg.cxx +++ b/sw/source/uibase/misc/redlndlg.cxx @@ -1143,7 +1143,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; @@ -1190,7 +1190,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) @@ -1205,7 +1205,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) ); }
