sw/qa/extras/tiledrendering/tiledrendering.cxx | 10 +++++----- sw/source/core/doc/DocumentRedlineManager.cxx | 23 +++++++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-)
New commits: commit 7395f88e4b79a83af66692303184d905a304b370 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Oct 19 20:32:56 2023 +0300 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Oct 20 10:51:33 2023 +0200 Do not forget to notify in cases when modified redline's start/end don't move SwRangeRedline::SetStart/SetEnd call MaybeNotifyRedlineModification, which means that e.g. continuing typing at the end of the current redline would continue sending notifications. But as soon as cursor is moved (e.g. using arrow left, inside the current redline), the next modification is completely inside the current one, and would be simply deleted. In this case (and other similar), notifications weren't sent. This change tries to make sure that every such case is handled. In case of delete-inside-insert, when positions are equal, the current redline is deleted; MaybeNotifyRedlineModification is not called, to not crash. I don't know if this needs additional notification. Change-Id: I2a97354623a5a784928ce4e1f2c93606954bd11c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158198 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> (cherry picked from commit 34ac12dca3f5af50fddfb7c77e2943897980b815) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158163 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx b/sw/qa/extras/tiledrendering/tiledrendering.cxx index 91344a563505..4f70ac36a87f 100644 --- a/sw/qa/extras/tiledrendering/tiledrendering.cxx +++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx @@ -1690,9 +1690,9 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testRedlineUpdateCallback) m_nRedlineTableEntryModified = 0; pWrtShell->DelLeft(); - // Assert that we get exactly one notification about the redline update. + // Assert that we get exactly two notification about the redline update. // This was 0, as LOK_CALLBACK_REDLINE_TABLE_ENTRY_MODIFIED wasn't sent. - CPPUNIT_ASSERT_EQUAL(1, m_nRedlineTableEntryModified); + CPPUNIT_ASSERT_EQUAL(2, m_nRedlineTableEntryModified); // Turn off the change tracking mode, make some modification to left of the // redline so that its position changes @@ -1701,18 +1701,18 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testRedlineUpdateCallback) pWrtShell->Insert("This text is left of the redline"); // Position of the redline has changed => Modify callback - CPPUNIT_ASSERT_EQUAL(2, m_nRedlineTableEntryModified); + CPPUNIT_ASSERT_EQUAL(3, m_nRedlineTableEntryModified); pWrtShell->DelLeft(); // Deletion also emits Modify callback - CPPUNIT_ASSERT_EQUAL(3, m_nRedlineTableEntryModified); + CPPUNIT_ASSERT_EQUAL(4, m_nRedlineTableEntryModified); // Make changes to the right of the redline => no position change in redline pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 100/*Go enough right */, /*bBasicCall=*/false); pWrtShell->Insert("This text is right of the redline"); // No Modify callbacks - CPPUNIT_ASSERT_EQUAL(3, m_nRedlineTableEntryModified); + CPPUNIT_ASSERT_EQUAL(4, m_nRedlineTableEntryModified); } CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testGetViewRenderState) diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx index c7ce853445cc..f538e5d88a81 100644 --- a/sw/source/core/doc/DocumentRedlineManager.cxx +++ b/sw/source/core/doc/DocumentRedlineManager.cxx @@ -1383,6 +1383,7 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall !pRedl->IsMoved() ) { bool bDelete = false; + bool bMaybeNotify = false; // Merge if applicable? if( (( SwComparePosition::Behind == eCmpPos && @@ -1433,22 +1434,22 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall *pStt = *pREnd; if( ( *pStt == *pEnd ) && ( pNewRedl->GetContentIdx() == nullptr ) ) - bDelete = true; + bDelete = bMaybeNotify = true; } else if( SwComparePosition::OverlapBefore == eCmpPos ) { *pEnd = *pRStt; if( ( *pStt == *pEnd ) && ( pNewRedl->GetContentIdx() == nullptr ) ) - bDelete = true; + bDelete = bMaybeNotify = true; } else if( SwComparePosition::Inside == eCmpPos ) { - bDelete = true; + bDelete = bMaybeNotify = true; bMerged = true; } else if( SwComparePosition::Equal == eCmpPos ) - bDelete = true; + bDelete = bMaybeNotify = true; if( bDelete ) { @@ -1456,6 +1457,9 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall pNewRedl = nullptr; bCompress = true; + if (bMaybeNotify) + MaybeNotifyRedlineModification(*pRedl, m_rDoc); + // set IsMoved checking nearby redlines if (n < maRedlineTable.size()) // in case above 're-insert' failed maRedlineTable.isMoved(n); @@ -1517,6 +1521,8 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall delete pNewRedl; pNewRedl = nullptr; bCompress = true; + + MaybeNotifyRedlineModification(*pRedl, m_rDoc); } } break; @@ -1666,6 +1672,8 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall delete pNewRedl; pNewRedl = nullptr; bCompress = true; + + MaybeNotifyRedlineModification(*pRedl, m_rDoc); break; case SwComparePosition::OverlapBefore: @@ -1789,6 +1797,9 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall } delete pNewRedl; pNewRedl = nullptr; + + if (!bDec) + MaybeNotifyRedlineModification(*pRedl, m_rDoc); break; case SwComparePosition::Outside: @@ -2086,6 +2097,8 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall case SwComparePosition::Inside: delete pNewRedl; pNewRedl = nullptr; + + MaybeNotifyRedlineModification(*pRedl, m_rDoc); break; case SwComparePosition::Outside: @@ -2133,6 +2146,8 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall // own one can be ignored completely delete pNewRedl; pNewRedl = nullptr; + + MaybeNotifyRedlineModification(*pRedl, m_rDoc); } else if( *pREnd == *pEnd ) // or else only shorten the current one
