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 d8713e13706ceb8046d1b5adf58a817ffbad72f8 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Nov 30 17:53:26 2023 +0300 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Dec 1 08:16:15 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 <[email protected]> (cherry picked from commit 9c22a72a2fc92146d24c6b673d9c07b1e6ff83f2) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160048 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> 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 cedac55734c8..f35aeb0cf6ef 100644 --- a/sw/qa/extras/uiwriter/uiwriter8.cxx +++ b/sw/qa/extras/uiwriter/uiwriter8.cxx @@ -2636,6 +2636,33 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest8, testTdf73483) assertXPath(pXml, para_style_path, "master-page-name", "Right_20_Page"); } +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 e2ba9748c503..4922c802b501 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 );
