From 692bc46b25db61176b4ced7b7beffeca7d55068e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20N=C3=A9meth?= Date: Fri, 10 Dec 2021 10:37:33 +0100 Subject: [PATCH] tdf#146140 sw DOCX import: fix moveFrom regression with broken text content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Also a small clean-up: increase the character limit for tracked text moving detection: Only 2 or more (non-whitespace) character deletions are checked for it, because single characters are often typos or some control-like characters, e.g. soft hyphen, not real text movings.) Details of the regression: commit d32d9a2b3c5e3963f4a18f6c7bbf50fab2e9b2be "tdf#123460 DOCX track changes: moveFrom completely" fixed the missing redline import of the end of the moved paragraphs, but paragraph end was imported as w:del, not w:moveFrom explicitly. From commit f51fa7534421a195a58b4a737a2e836d8c25ba81 "tdf#145718 sw, DOCX import: complete tracked text moving" this resulted two deletions (a moved one and a plain one) instead of the previous single one. Moreover, exporting these double deletions at the same position to ODT, raised a back-compatibility issue with broken text content, see tdf#107292 (solved recently, but not in older LibreOffice versions). Removing the explicit w:del code path in writerfilter, it solved the regression from commit f51fa7534421a195a58b4a737a2e836d8c25ba81 "tdf#145718 sw, DOCX import: complete tracked text moving". See also commit 9e1e88ad5cf2dc0e9b188c60930445652a6c7519 "tdf#145720 DOCX export: fix loss of tracked moving". Change-Id: I15bfc83b87dd42a762ff84edf5bae765fe02a5ae Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126631 Tested-by: Jenkins Reviewed-by: László Németh --- sw/qa/extras/layout/layout2.cxx | 2 +- sw/qa/extras/ooxmlexport/ooxmlexport13.cxx | 33 +++++++++++++++++-- sw/source/core/doc/docredln.cxx | 2 +- .../source/dmapper/DomainMapper_Impl.cxx | 1 - 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/sw/qa/extras/layout/layout2.cxx b/sw/qa/extras/layout/layout2.cxx index 92910435383e..10d01fa05e33 100644 --- a/sw/qa/extras/layout/layout2.cxx +++ b/sw/qa/extras/layout/layout2.cxx @@ -337,7 +337,7 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testRedlineMovingDOCX) SwEditShell* const pEditShell(pDoc->GetEditShell()); // This was 2 (moveFrom and moveTo joined other redlines) - CPPUNIT_ASSERT_EQUAL(static_cast(6), pEditShell->GetRedlineCount()); + CPPUNIT_ASSERT_EQUAL(static_cast(5), pEditShell->GetRedlineCount()); // Dump the rendering of the first page as an XML file. std::shared_ptr xMetaFile = pShell->GetPreviewMetaFile(); diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx index 6600928f37e9..609d3f7f30b6 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx @@ -766,10 +766,37 @@ DECLARE_OOXMLEXPORT_TEST(testTdf123460, "tdf123460.docx") CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 2), "RedlineType")); CPPUNIT_ASSERT_EQUAL(OUString("Delete"),getProperty(getRun(getParagraph(2), 2), "RedlineType")); CPPUNIT_ASSERT_EQUAL(true, getRun( getParagraph( 2 ), 3 )->getString().endsWith("tellus.")); + CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 4), "Bookmark")); // deleted paragraph mark at the end of the second paragraph - CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 5), "RedlineType")); - CPPUNIT_ASSERT_EQUAL(OUString("Delete"),getProperty(getRun(getParagraph(2), 5), "RedlineType")); - CPPUNIT_ASSERT_EQUAL( OUString( "" ), getRun( getParagraph( 2 ), 6 )->getString()); + if (mbExported) + { + // there is no run after the MoveBookmark + bool bCaught = false; + try + { + getRun( getParagraph( 2 ), 5 ); + } + catch (container::NoSuchElementException&) + { + bCaught = true; + } + CPPUNIT_ASSERT_EQUAL(true, bCaught); + } +} + +CPPUNIT_TEST_FIXTURE(Test, testTdf146140) +{ + loadAndSave("tdf123460.docx"); + CPPUNIT_ASSERT_EQUAL(1, getPages()); + xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml"); + CPPUNIT_ASSERT(pXmlDoc); + + // This was 1 (put end of paragraph of the previous moveFrom into a w:del, + // resulting double deletions at the same position, which is an + // ODT back-compatibility issue described in tdf#107292) + assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:pPr/w:rPr/w:del", 0); + // This was 0 + assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:pPr/w:rPr/w:moveFrom", 1); } //tdf#125298: fix charlimit restrictions in bookmarknames and field references if they contain non-ascii characters diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx index 28d7295fe344..96f3742ce8f1 100644 --- a/sw/source/core/doc/docredln.cxx +++ b/sw/source/core/doc/docredln.cxx @@ -802,7 +802,7 @@ bool SwRedlineTable::isMoved( size_type rPos ) const } const OUString sTrimmed = pPaM->GetText().trim(); - if ( sTrimmed.isEmpty() ) + if ( sTrimmed.getLength() < 2 ) { if ( bDeletePaM ) delete pPaM; diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index befa25aed954..5662e8967d8b 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -3119,7 +3119,6 @@ void DomainMapper_Impl::CheckParaMarkerRedline( uno::Reference< text::XTextRange else if ( m_pParaMarkerRedlineMoveFrom ) { // terminating moveFrom redline removes also the paragraph mark - m_pParaMarkerRedlineMoveFrom->m_nToken = XML_del; CreateRedline( xRange, m_pParaMarkerRedlineMoveFrom ); } if ( m_pParaMarkerRedlineMoveFrom )