tdf#159815 DOCX import: fix redlined to-char image followed by inline SDT

The 2nd para of the bugdoc has a leading word, then a redlined anchored
shape, finally a redlined checkbox content control. The Writer import
result's content control starts at para start, while it should only
start after the anchored shape.

What happens is that writerfilter::dmapper::DomainMapper_Impl::PushSdt()
gets called to remember the SDT start, then
DomainMapper_Impl::applyToggleAttributes() deletes some content since
commit 8726cf692299ea262a7455adcf6ec25451c7869d (tdf#142700 DOCX: fix
lost track changes of images, 2021-07-08), which invalidates the SDT
start position, finally PopSdt gets called, but that fails because our
start position is no longer valid. This used to abort the import
process. Since commit 1b0f67018fa1d514ebca59e081efdd24c1d7811b (docx
import: correct redline content-controls, 2024-02-20), the import
finishes the but start of the SDT is incorrect: it's at the para start,
while it should start in the middle of the paragraph. The direct reason
for the invalidation is a call to SwXTextRange::Impl::Notify(), which is
from a content deletion from applyToggleAttributes().

Fix the problem by not deleting content when we're past PushSdt() and we
haven't called PopSdt(): extract the redlined anchored shape code into a
new DomainMapper_Impl::MergeAtContentImageRedlineWithNext() and call
that also in DomainMapper_Impl::PushSdt() early, so it won't be called
when we're in the middle of an SDT.

This way createTextCursorByRange() should not fail, so warn in case it
still fails in DomainMapper_Impl::PopSdt().

Change-Id: Ic4198804a92088ec268203d44c0da2d6997754b7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163678
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

1 files changed, 42 insertions, 33 deletions

diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index d5c6d553ea11..f73504824ea0 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -984,6 +984,10 @@ void DomainMapper_Impl::PushSdt()
         return;
     }
 
+    // This may delete text, so call it early, before we would set our start position, which may be
+    // invalidated by a delete.
+    MergeAtContentImageRedlineWithNext(xTextAppend);
+
     uno::Reference<text::XText> xText = xTextAppend->getText();
     if (!xText.is())
     {
@@ -1022,6 +1026,7 @@ void DomainMapper_Impl::PopSdt()
     }
     catch (const uno::RuntimeException&)
     {
+        TOOLS_WARN_EXCEPTION("writerfilter", "DomainMapper_Impl::DomainMapper_Impl::PopSdt: createTextCursorByRange() failed");
         // We redline form controls and that gets us confused when
         // we process the SDT around the placeholder. What seems to
         // happen is we lose the text-range when we pop the SDT position.
@@ -3187,6 +3192,42 @@ void DomainMapper_Impl::applyToggleAttributes(const PropertyMapPtr& pPropertyMap
     }
 }
 
+void DomainMapper_Impl::MergeAtContentImageRedlineWithNext(const css::uno::Reference<css::text::XTextAppend>& xTextAppend)
+{
+    // 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;
+    }
+}
 
     void DomainMapper_Impl::appendTextPortion( const OUString& rString, const PropertyMapPtr& pPropertyMap )
 {
@@ -3220,39 +3261,7 @@ void DomainMapper_Impl::applyToggleAttributes(const PropertyMapPtr& pPropertyMap
                     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;
-        }
+        MergeAtContentImageRedlineWithNext(xTextAppend);
 
         uno::Reference< text::XTextRange > xTextRange;
         if (m_aTextAppendStack.top().xInsertPosition.is())