sw/qa/uitest/writer_tests/trackedChanges.py | 4 ++--
sw/source/core/doc/docredln.cxx | 20 +++++++++++++++-----
2 files changed, 17 insertions(+), 7 deletions(-)
New commits:
commit a1411f0fb19981d3c156d34ace5db10bd5d22aec
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Fri Oct 21 20:57:14 2022 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Fri Oct 21 23:36:20 2022 +0200
Improve SwRedlineData::CanCombine date comparison
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 <sal/config.h>
> +
> +#include <chrono>
> +
> #include <com/sun/star/drawing/FillStyle.hpp>
> #include <swmodeltestbase.hxx>
> #include <cntfrm.hxx>
> @@ -33,6 +37,7 @@
> #include <com/sun/star/text/XTextViewCursorSupplier.hpp>
> #include <com/sun/star/view/XSelectionSupplier.hpp>
> #include <o3tl/cppunittraitshelper.hxx>
> +#include <osl/thread.hxx>
> #include <swdtflvr.hxx>
> #include <comphelper/propertysequence.hxx>
> #include <LibreOfficeKit/LibreOfficeKitEnums.h>
> @@ -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 <text:tracked-changes> <dc:date> 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 <sberg...@redhat.com>
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
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 9749c94331fb..8904e8b14d4a 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -47,6 +47,8 @@
#include <hints.hxx>
#include <pamtyp.hxx>
#include <poolfmt.hxx>
+#include <algorithm>
+#include <limits>
#include <utility>
#include <view.hxx>
#include <viewopt.hxx>
@@ -1045,16 +1047,24 @@ SwRedlineData::~SwRedlineData()
delete m_pNext;
}
+// Check whether the absolute difference between the two dates is no larger
than one minute (can
+// give inaccurate results if at least one of the dates is not
valid/normalized):
+static bool deltaOneMinute(DateTime const & t1, DateTime const & t2) {
+ auto const & [min, max] = std::minmax(t1, t2);
+ // Avoid overflow of `min + tools::Time(0, 1)` below when min is close to
the maximum valid
+ // DateTime:
+ if (min >= DateTime({31, 12, std::numeric_limits<sal_Int16>::max()}, {23,
59})) {
+ return true;
+ }
+ return max <= min + tools::Time(0, 1);
+}
+
bool SwRedlineData::CanCombine(const SwRedlineData& rCmp) const
{
- DateTime aTime = GetTimeStamp();
- aTime.SetSec(0);
- DateTime aCompareTime = rCmp.GetTimeStamp();
- aCompareTime.SetSec(0);
return m_nAuthor == rCmp.m_nAuthor &&
m_eType == rCmp.m_eType &&
m_sComment == rCmp.m_sComment &&
- aTime == aCompareTime &&
+ deltaOneMinute(GetTimeStamp(), rCmp.GetTimeStamp()) &&
m_bMoved == rCmp.m_bMoved &&
(( !m_pNext && !rCmp.m_pNext ) ||
( m_pNext && rCmp.m_pNext &&
