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 55640f9f0a4741f8e4b5b98096af822cee71da2c Author: Miklos Vajna <[email protected]> AuthorDate: Mon Sep 29 08:44:22 2025 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Sep 29 11:24:57 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]> 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 990bb7adc07a..b631a1eb5e4a 100644 --- a/sw/inc/editsh.hxx +++ b/sw/inc/editsh.hxx @@ -958,8 +958,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 45cd8ad14793..6ad517f34d5e 100644 --- a/sw/source/core/edit/edredln.cxx +++ b/sw/source/core/edit/edredln.cxx @@ -85,11 +85,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(); @@ -172,11 +172,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 b07ff30c3c3d..4fd96b5a80ab 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, 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 063e70c925fe..4e5609886585 100644 --- a/sw/source/uibase/misc/redlndlg.cxx +++ b/sw/source/uibase/misc/redlndlg.cxx @@ -1147,7 +1147,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; @@ -1194,7 +1194,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) @@ -1209,7 +1209,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) ); }
