sw/qa/core/doc/doc.cxx | 17 ++++++++++++++ sw/source/core/doc/DocumentRedlineManager.cxx | 30 ++++++++++++++++++-------- 2 files changed, 38 insertions(+), 9 deletions(-)
New commits: commit d659f4b12a24e275e38ec5aa7406c32c9fa08c48 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Fri May 16 09:41:01 2025 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Fri May 16 14:03:33 2025 +0200 tdf#166319 sw interdependent redlines: handle reject of delete under format The bugdoc has <del>AA<format>BB</format>CC</del> in it, and once the cursor is in the middle of the AA, then pressing Reject should get rid of the delete and keep the format, but it resulted in no redlines and a "AACC" string. What happened is that we found the delete under format, but the action was still reject, and it's OK to implement reject by deleting the content for insert-then-format, but this won't work for delete-then-format. Fix the problem by reusing lcl_AcceptInnerInsertRedline() for this purpose: accepting an underlying insert is effectively a NOP (body text doesn't change), and rejecting a delete is in the same category. Rename to lcl_DeleteInnerRedline(), because what this function does is to get rid of "underlying" (earlier) redlines of a redline, so this is useful for rejecting deletes as well, not just for accepting inserts. Change-Id: Ie2db419844b77638d12fa6edfcbc0d5c3b59ad20 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185404 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx index e7af522adeeb..62ddfb9bab26 100644 --- a/sw/qa/core/doc/doc.cxx +++ b/sw/qa/core/doc/doc.cxx @@ -815,6 +815,23 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testDelThenFormat) SwTextNode* pTextNode = pWrtShell->GetCursor()->GetPointNode().GetTextNode(); // Also make sure the text of the format-on-delete redline is removed. CPPUNIT_ASSERT(pTextNode->GetText().isEmpty()); + + // And when rejecting the delete: + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(u"AAABBBCCC"_ustr, pTextNode->GetText()); + pWrtShell->RejectRedline(0); + + // Then make sure the delete goes away, but the text node string and the format redline stays: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 0 + // i.e. the format redline on top of the delete was lost. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rRedlines.size()); + CPPUNIT_ASSERT_EQUAL(RedlineType::Format, rRedlines[0]->GetType()); + const SwRedlineData& rRedlineData = rRedlines[0]->GetRedlineData(); + CPPUNIT_ASSERT(!rRedlineData.Next()); + // This was AAACCC. + CPPUNIT_ASSERT_EQUAL(u"AAABBBCCC"_ustr, pTextNode->GetText()); } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index cd25b95ab5c5..b50d07a5b263 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -997,7 +997,9 @@ namespace return bRet; } - bool lcl_AcceptInnerInsertRedline(const SwRedlineTable& rArr, const SwRedlineTable::size_type& rPos, + /// Given a redline that has an other underlying redline, drop that underlying redline. + /// Used to accept an insert or rejecting a delete, i.e. no changes to the text node strings. + bool lcl_DeleteInnerRedline(const SwRedlineTable& rArr, const SwRedlineTable::size_type& rPos, int nDepth) { SwRangeRedline* pRedl = rArr[rPos]; @@ -3312,7 +3314,7 @@ bool DocumentRedlineManager::AcceptRedlineRange(SwRedlineTable::size_type nPosOr else { // we should leave the other type of redline, and only accept the inner insert. - bRet |= lcl_AcceptInnerInsertRedline(maRedlineTable, nRdlIdx, 1); + bRet |= lcl_DeleteInnerRedline(maRedlineTable, nRdlIdx, 1); } nRdlIdx++; //we will decrease it in the loop anyway. } @@ -3342,7 +3344,7 @@ bool DocumentRedlineManager::AcceptMovedRedlines(sal_uInt32 nMovedID, bool bCall if (pTmp->GetMoved(0) == nMovedID) bRet |= lcl_AcceptRedline(maRedlineTable, nRdlIdx, bCallDelete); else - bRet |= lcl_AcceptInnerInsertRedline(maRedlineTable, nRdlIdx, 1); + bRet |= lcl_DeleteInnerRedline(maRedlineTable, nRdlIdx, 1); nRdlIdx++; //we will decrease it in the loop anyway. } @@ -3482,7 +3484,7 @@ bool DocumentRedlineManager::AcceptRedline( const SwPaM& rPam, bool bCallDelete, // For now it is called only if it is an Insert redline in a delete redline. SwRedlineTable::size_type nRdlIdx = 0; maRedlineTable.FindAtPosition(*rPam.Start(), nRdlIdx); - if (lcl_AcceptInnerInsertRedline(maRedlineTable, nRdlIdx, 1)) + if (lcl_DeleteInnerRedline(maRedlineTable, nRdlIdx, 1)) nRet = 1; } if( nRet > 0 ) @@ -3582,9 +3584,7 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr else if (CanCombineTypesForAcceptReject(aOrigData, *pTmp) && pTmp->GetRedlineData(1).CanCombineForAcceptReject(aOrigData)) { - // The Insert redline we want to reject has another type of redline too - // without the insert, the other type is meaningless - // so we rather just accept the other type of redline + // The Insert/Delete redline we want to reject has another type of redline too if (m_rDoc.GetIDocumentUndoRedo().DoesUndo()) { std::unique_ptr<SwUndoAcceptRedline> pUndoRdl @@ -3597,13 +3597,25 @@ bool DocumentRedlineManager::RejectRedlineRange(SwRedlineTable::size_type nPosOr nPamEndtNI = pTmp->Start()->GetNodeIndex(); nPamEndCI = pTmp->Start()->GetContentIndex(); std::optional<SwPaM> oPam; + RedlineType eInnerType = aOrigData.GetType(); RedlineType eOuterType = pTmp->GetType(); - if (eOuterType == RedlineType::Format) + if (eInnerType == RedlineType::Insert && eOuterType == RedlineType::Format) { // The accept won't implicitly delete the range, so track its boundaries. oPam.emplace(*pTmp->Start(), *pTmp->End()); } - bRet |= lcl_AcceptRedline(maRedlineTable, nRdlIdx, bCallDelete); + + if (eInnerType == RedlineType::Delete && eOuterType == RedlineType::Format) + { + // Keep the outer redline, just get rid of the underlying delete. + bRet |= lcl_DeleteInnerRedline(maRedlineTable, nRdlIdx, 1); + } + else + { + // without the insert, the other type is meaningless + // so we rather just accept the other type of redline + bRet |= lcl_AcceptRedline(maRedlineTable, nRdlIdx, bCallDelete); + } if (oPam) { // Handles undo/redo itself.