tdf#142700 DOCX: fix lost track changes of images

anchored to characters or paragraphs.

Tracked deletions and insertions weren't imported,
if they contain only images anchored to character,
resulting loss of the document content: i.e. deleted
images were reappeared, as not deleted images.

Note: because change tracking of the OpenDocument
and Writer supports only text range based changes,
the fix is a workaround using zero width spaces
to create an invisible text around the anchoring point
of the images. This workaround is not used, if it's not
necessary, i.e. if the image is part of a bigger
deletion or insertion, which contains also text, not
only an image.

Change-Id: Iaae6aae2c01191512c71117a0c788a4147c4cae0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118557
Tested-by: Jenkins
Reviewed-by: László Németh <nemeth@numbertext.org>

4 files changed, 78 insertions, 1 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf142700.docx b/sw/qa/extras/ooxmlexport/data/tdf142700.docx
new file mode 100644
index 000000000000..393e2ff4771b
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf142700.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index 9cfaf5aa6e6c..1864c4bcf83e 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -1376,6 +1376,14 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128913, "tdf128913.docx")
     assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:del/w:r/w:drawing/wp:inline/a:graphic");
 }
 
+DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf142700, "tdf142700.docx")
+{
+    xmlDocUniquePtr pXmlDoc = parseExport();
+    // w:ins and w:del are imported correctly, if they contain only images anchored to character
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:ins/w:r/w:drawing/wp:anchor/a:graphic");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:del/w:r/w:drawing/wp:anchor/a:graphic");
+}
+
 DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf142387, "tdf142387.docx")
 {
     xmlDocUniquePtr pXmlDoc = parseExport();
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 30928027007c..25390cc20024 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -316,6 +316,7 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_bLineNumberingSet( false ),
         m_bIsInFootnoteProperties( false ),
         m_bIsParaMarkerChange( false ),
+        m_bRedlineImageInPreviousRun( false ),
         m_bParaChanged( false ),
         m_bIsFirstParaInSection( true ),
         m_bIsFirstParaInSectionAfterRedline( true ),
@@ -2245,6 +2246,40 @@ void DomainMapper_Impl::appendTextPortion( const OUString& rString, const Proper
                     rValue.Value <<= false;
             }
 
+        // remove workaround for change tracked images, if they are part of a redline,
+        // i.e. if the next run is a tracked change with the same type, author and date,
+        // as in the change tracking of the image.
+        if ( m_bRedlineImageInPreviousRun )
+        {
+            auto pCurrentRedline = m_aRedlines.top().size() > 0
+                    ? m_aRedlines.top().back()
+                    : GetTopContextOfType(CONTEXT_CHARACTER) &&
+                                GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().size() > 0
+                        ? GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().back()
+                        : nullptr;
+            if ( m_previousRedline && pCurrentRedline &&
+                   // same redline
+                   (m_previousRedline->m_nToken & 0xffff) == (pCurrentRedline->m_nToken & 0xffff) &&
+                    m_previousRedline->m_sAuthor == pCurrentRedline->m_sAuthor &&
+                    m_previousRedline->m_sDate == pCurrentRedline->m_sDate )
+            {
+                uno::Reference< text::XTextCursor > xCursor = xTextAppend->getEnd()->getText( )->createTextCursor( );
+                assert(xCursor.is());
+                xCursor->gotoEnd(false);
+                xCursor->goLeft(2, true);
+                if ( xCursor->getString() == u"​​" )
+                {
+                    xCursor->goRight(1, true);
+                    xCursor->setString("");
+                    xCursor->gotoEnd(false);
+                    xCursor->goLeft(1, true);
+                    xCursor->setString("");
+                }
+            }
+
+            m_bRedlineImageInPreviousRun = false;
+        }
+
         uno::Reference< text::XTextRange > xTextRange;
         if (m_aTextAppendStack.top().xInsertPosition.is())
         {
@@ -7153,7 +7188,36 @@ void  DomainMapper_Impl::ImportGraphic(const writerfilter::Reference< Properties
     OSL_ENSURE( xTextContent.is(), "DomainMapper_Impl::ImportGraphic");
     if( xTextContent.is())
     {
-        appendTextContent( xTextContent, uno::Sequence< beans::PropertyValue >() );
+        bool bAppend = true;
+        // workaround for images anchored to characters: add ZWSPs around the anchoring point
+        if ( eGraphicImportType != IMPORT_AS_DETECTED_INLINE && !m_aRedlines.top().empty() )
+        {
+            uno::Reference< text::XTextAppend > xTextAppend = m_aTextAppendStack.top().xTextAppend;
+            if(xTextAppend.is())
+            {
+                try
+                {
+                    uno::Reference< text::XText > xText = xTextAppend->getText();
+                    uno::Reference< text::XTextCursor > xCrsr = xText->createTextCursor();
+                    xCrsr->gotoEnd(false);
+                    PropertyMapPtr pEmpty(new PropertyMap());
+                    appendTextPortion(u"​", pEmpty);
+                    appendTextContent( xTextContent, uno::Sequence< beans::PropertyValue >() );
+                    bAppend = false;
+                    xCrsr->gotoEnd(false);
+                    appendTextPortion(u"​", pEmpty);
+
+                    m_bRedlineImageInPreviousRun = true;
+                    m_previousRedline = m_currentRedline;
+                }
+                catch( const uno::Exception& )
+                {
+                }
+            }
+        }
+
+        if ( bAppend )
+            appendTextContent( xTextContent, uno::Sequence< beans::PropertyValue >() );
 
         if (eGraphicImportType == IMPORT_AS_DETECTED_ANCHOR && !m_aTextAppendStack.empty())
         {
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 845e902a9ee3..c89e24cd4329 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -569,6 +569,11 @@ private:
     bool                            m_bIsParaMarkerChange;
     // redline data of the terminating run, if it's a moveFrom deletion
     RedlineParamsPtr                m_pParaMarkerRedlineMoveFrom;
+    // This is for removing workaround (double ZWSPs around the anchoring point) for track
+    // changed images anchored *to* character, if it's followed by a redline text run immediately.
+    // (In that case, the image is part of a tracked text range, no need for the dummy
+    // text ZWSPs to keep the change tracking of the image in Writer.)
+    bool                            m_bRedlineImageInPreviousRun;
 
     /// If the current paragraph has any runs.
     bool                            m_bParaChanged;