From a1411f0fb19981d3c156d34ace5db10bd5d22aec Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Fri, 21 Oct 2022 20:57:14 +0200 Subject: Improve SwRedlineData::CanCombine date comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f240f073d228e62afd3f60563c23626efad0df7f "Related: tdf#102308 sw: ignore seconds when combining redlines" had just naively cut off the seconds for comparison, but which apparently gave unexpected results when the two dates are sufficiently close but lie on different sides of a minute tick: Autocorrection-related unit tests like CppunitTest_sw_uiwriter6 CPPUNIT_TEST_NAME=testRedlineAutoCorrect2::TestBody sporadically failed to execute some autocorrection, as in > uiwriter6.cxx:1436:Assertion > Test name: testRedlineAutoCorrect2::TestBody > equality assertion failed > - Expected: Lorem,... Lorem,… > - Actual : Lorem,... Lorem,... What happened there in testRedlineAutoCorrect2 (sw/qa/extras/uiwriter/uiwriter6.cxx) is that each emulateTyping consecutive simulated character input should extend the current redline range, by hitting the > // Merge if applicable? > if( (( SwComparePosition::Behind == eCmpPos && > IsPrevPos( *pREnd, *pStt ) ) || > ( SwComparePosition::CollideStart == eCmpPos ) || > ( SwComparePosition::OverlapBehind == eCmpPos ) ) && > pRedl->CanCombine( *pNewRedl ) && > ( n+1 >= maRedlineTable.size() || > ( *maRedlineTable[ n+1 ]->Start() >= *pEnd && > *maRedlineTable[ n+1 ]->Start() != *pREnd ) ) ) branch in DocumentRedlineManager::AppendRedline (sw/source/core/doc/DocumentRedlineManager.cxx). However, when that happened to fail on a minute tick boundary because its > pRedl->CanCombine( *pNewRedl ) && part was false, we would end up with split redline ranges, and SwAutoCorrDoc::ChgAutoCorrWord (sw/source/core/edit/acorrect.cxx) would not execute the autocorrection replacement across those split ranges due to its > // don't replace, if a redline starts or ends within the original text code. (One way of making that test fail reproducibly would be with some > diff --git a/sw/qa/extras/uiwriter/uiwriter6.cxx b/sw/qa/extras/uiwriter/uiwriter6.cxx > index 0159c5bf07ed..8716c6a09882 100644 > --- a/sw/qa/extras/uiwriter/uiwriter6.cxx > +++ b/sw/qa/extras/uiwriter/uiwriter6.cxx > @@ -7,6 +7,10 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. > */ > > +#include > + > +#include > + > #include > #include > #include > @@ -33,6 +37,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1431,7 +1436,9 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest6, testRedlineAutoCorrect2) > CPPUNIT_ASSERT_EQUAL(sReplaced, getParagraph(1)->getString()); > > // Continue it: > - emulateTyping(rXTextDocument, u"Lorem,... "); > + emulateTyping(rXTextDocument, u"Lorem,..."); > + osl::Thread::wait(std::chrono::seconds(70)); > + emulateTyping(rXTextDocument, u" "); > sReplaced = u"Lorem,... Lorem,… "; > CPPUNIT_ASSERT_EQUAL(sReplaced, getParagraph(1)->getString()); > } patch.) So at least improve SwRedlineData::CanCombine to properly check for a date delta of <= 1 minute. However, that still leaves those tests prone to sporadic failures (even though less likely), if they get delayed for a sufficient amount of time in strategically unfortunate places. Ideally, those autocorrection-related tests (which appear to be the ones in sw/qa/extras/uiwriter/uiwriter6.cxx using emulateTyping) would run with redlining turned off, but many of them explicitly enable redlining, presumably for a reason. With that change, UITest_writer_tests started to fail with > FAIL: test_tdf135018 (trackedChanges.trackedchanges) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "sw/qa/uitest/writer_tests/trackedChanges.py", line 203, in test_tdf135018 > self.assertEqual(147, len(changesList.getChildren())) > AssertionError: 147 != 111 in a test introduced with b6ab2330d97672936edc56de8d6f5b6f772908ff "tdf#135018: sw: Add UItest". That sw/qa/uitest/data/tdf135018.odt's content.xml contains 147 elements ranging from 2020-07-11T10:29:12 to 2020-07-21T11:50:32, and apparently some adjacent changes with sufficiently close dates now started to get combined when executing .uno:AcceptTrackedChanges. Change-Id: I1d3453bbf5b0206c080bd3421d525e6b8351f2b7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141653 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- sw/qa/uitest/writer_tests/trackedChanges.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sw/qa') diff --git a/sw/qa/uitest/writer_tests/trackedChanges.py b/sw/qa/uitest/writer_tests/trackedChanges.py index 3225a74f6e08..592c839fcd18 100644 --- a/sw/qa/uitest/writer_tests/trackedChanges.py +++ b/sw/qa/uitest/writer_tests/trackedChanges.py @@ -200,7 +200,7 @@ class trackedchanges(UITestCase): with self.ui_test.execute_modeless_dialog_through_command(".uno:AcceptTrackedChanges", close_button="close") as xTrackDlg: changesList = xTrackDlg.getChild("writerchanges") - self.assertEqual(147, len(changesList.getChildren())) + self.assertEqual(111, len(changesList.getChildren())) # Without the fix in place, it would have crashed here xAccBtn = xTrackDlg.getChild("acceptall") @@ -211,7 +211,7 @@ class trackedchanges(UITestCase): xUndoBtn = xTrackDlg.getChild("undo") xUndoBtn.executeAction("CLICK", tuple()) - self.assertEqual(147, len(changesList.getChildren())) + self.assertEqual(111, len(changesList.getChildren())) # Check the changes are shown after opening the Manage Tracked Changes dialog -- cgit