sw/qa/extras/uiwriter/data/tdf158459_tracked_changes_across_nodes.fodt | 45
++++++++++
sw/qa/extras/uiwriter/uiwriter8.cxx | 27
++++++
sw/source/core/doc/DocumentContentOperationsManager.cxx | 9
+-
3 files changed, 79 insertions(+), 2 deletions(-)
New commits:
commit 9c22a72a2fc92146d24c6b673d9c07b1e6ff83f2
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu Nov 30 17:53:26 2023 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Thu Nov 30 17:35:50 2023 +0100
tdf#158459: call DeleteAndJoin in strict reverse order
The crash was caused by removal of a paragraph break (pointed to by a
PaM returned by pDelPam->GetNext()), that resulted in merge of the node
pDelPam pointed to, with the previous node. The destructin of the node
made pDelPam dangle. In debug builds, an assertion failed in destructor
of SwContentIndexReg. I.e., since the code always called DeleteAndJoin
on pDelPam->GetNext(), the processing went from second-to-last deletion
towards the begin of the document, and the last deletion (represented
by pDelPam) was handled last, when GetNext call wrapped.
This change makes sure that the order is strict, from last to first.
Change-Id: I5cb7fe2f06d4138d3c445eeca8220f0a87a82626
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160158
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git
a/sw/qa/extras/uiwriter/data/tdf158459_tracked_changes_across_nodes.fodt
b/sw/qa/extras/uiwriter/data/tdf158459_tracked_changes_across_nodes.fodt
new file mode 100644
index 000000000000..bdd03bc0be70
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data/tdf158459_tracked_changes_across_nodes.fodt
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document
xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0"
xmlns:dc="http://purl.org/dc/elements/1.1/";
xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"
office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:body>
+ <office:text>
+ <text:tracked-changes text:track-changes="false">
+ <text:changed-region xml:id="ct1" text:id="ct1">
+ <text:deletion>
+ <office:change-info>
+ <dc:creator>x</dc:creator>
+ <dc:date>2000-01-01</dc:date>
+ </office:change-info>
+ </text:deletion>
+ </text:changed-region>
+ <text:changed-region xml:id="ct2" text:id="ct2">
+ <text:insertion>
+ <office:change-info>
+ <dc:creator>x</dc:creator>
+ <dc:date>2000-01-02</dc:date>
+ </office:change-info>
+ </text:insertion>
+ </text:changed-region>
+ <text:changed-region xml:id="ct3" text:id="ct3">
+ <text:deletion>
+ <office:change-info>
+ <dc:creator>x</dc:creator>
+ <dc:date>2000-01-03</dc:date>
+ </office:change-info>
+ </text:deletion>
+ </text:changed-region>
+ <text:changed-region xml:id="ct4" text:id="ct4">
+ <text:deletion>
+ <office:change-info>
+ <dc:creator>x</dc:creator>
+ <dc:date>2000-01-04</dc:date>
+ </office:change-info>
+ </text:deletion>
+ </text:changed-region>
+ </text:tracked-changes>
+ <text:p/>
+ <text:p>a<text:change-start text:change-id="ct1"/></text:p>
+ <text:p><text:change-end text:change-id="ct1"/><text:change-start
text:change-id="ct2"/>b<text:change-end
text:change-id="ct2"/><text:change-start
text:change-id="ct3"/>c<text:change-end
text:change-id="ct3"/>d<text:change-start
text:change-id="ct4"/>e<text:change-end text:change-id="ct4"/>f</text:p>
+ </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/uiwriter/uiwriter8.cxx
b/sw/qa/extras/uiwriter/uiwriter8.cxx
index 0fbb8e9fae81..641797b2c7b0 100644
--- a/sw/qa/extras/uiwriter/uiwriter8.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter8.cxx
@@ -2817,6 +2817,33 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf156560)
dispatchCommand(mxComponent, ".uno:InsertHeader", {});
}
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf158459)
+{
+ createSwDoc("tdf158459_tracked_changes_across_nodes.fodt");
+ SwDoc* pDoc = getSwDoc();
+
+ SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+ CPPUNIT_ASSERT(pWrtShell);
+ pWrtShell->FwdPara(); // Skip first paragraph
+ pWrtShell->EndOfSection(true); // Select everything to the end
+
+ SwDoc aClipboard;
+ pWrtShell->Copy(aClipboard); // This must not crash
+
+ pWrtShell->SelAll();
+ pWrtShell->Delete();
+ pWrtShell->Paste(aClipboard); // Replace everything with the copied stuff
+
+ SwNodes& rNodes = pDoc->GetNodes();
+ SwNodeIndex aIdx(rNodes.GetEndOfExtras());
+ SwContentNode* pContentNode = rNodes.GoNext(&aIdx);
+ CPPUNIT_ASSERT(pContentNode);
+ SwTextNode* pTextNode = pContentNode->GetTextNode();
+ CPPUNIT_ASSERT(pTextNode);
+ // Check that deleted parts (paragraph break, "c", "e") haven't been pasted
+ CPPUNIT_ASSERT_EQUAL(OUString("abdf"), pTextNode->GetText());
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx
b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index d7d15d279776..09e0a1233e5d 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -440,11 +440,16 @@ namespace
::sw::UndoGuard const undoGuard(rDestDoc.GetIDocumentUndoRedo());
+ // At this point, pDelPam points to the last of maybe several disjoint
selections, organized
+ // in reverse order in document (so every GetNext() returns a PaM
closer to document start,
+ // until wrap to pDelPam). Removal of the selections must be from last
in document to first,
+ // to avoid situations when another PaM in chain points into the node
that will be destroyed
+ // (joined to previous) by removal of the currently processed PaM.
do {
- rDestDoc.getIDocumentContentOperations().DeleteAndJoin(
*pDelPam->GetNext() );
+ rDestDoc.getIDocumentContentOperations().DeleteAndJoin(*pDelPam);
if( !pDelPam->IsMultiSelection() )
break;
- delete pDelPam->GetNext();
+ pDelPam.reset(pDelPam->GetNext());
} while( true );
rDestDoc.getIDocumentRedlineAccess().SetRedlineFlags_intern( eOld );
