tdf#129582 sw: fix copying of flys in header/footer in DOCX/RTF import

The problem is that the exception for writerfilter in
IsDestroyFrameAnchoredAtChar() and IsSelectFrameAnchoredAtPara() is
wrong in the case when the header/footer content is copied via
SwXText::copyText(); that is, previously the situation was that
writerfilter relied on Delete not deleting such flys (for
RemoveLastParagraph) but Copy copying them.

(regression from 28b77c89dfcafae82cf2a6d85731b643ff9290e5
and e75dd1fc992f168f24d66595265a978071cdd277)

So restrict the writerfilter hack to delete; this causes a problem with
ooxmlexport9 test testTdf100075: it has 2 flys anchored at the
same paragraph; writerfilter will insert the content into the body and
then convert to fly; when the 2nd one is converted it will copy the 1st
fly and anchor it inside the 2nd fly but then unotext.cxx:1719 will
reset its anchor to inside the body...

Prevent this unwanted copy by relying on the new parameter bCopyText
that was introduced in 04b2310aaa094794ceedaa1bb6ff1823a2d29d3e,
but change things a bit so that the case that pass in the extra flag
isn't the copyText() one that wants the *normal* selection semantics in
writerfilter import, but the 2 known places that want the *exceptional*
selection semantics in writerfilter import (hopefully there aren't more).

This is not ideal and the various bool parameters to CopyRange() plus
mbCopyIsMove plus mbIsRedlineMove should probably be consolidated
into some flags enum passed to CopyRange().

Change-Id: I638c7fa7ad0b4ec149aa6a1485e32f2c8e29ff5a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87072
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@cib.de>
(cherry picked from commit 81ec0039b2085faab49380c7a56af0c562d4c9e4)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87095
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

6 files changed, 33 insertions, 32 deletions

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index ea6ca925dadd..89013a7dd2f5 100644
--- a/sw/inc/undobj.hxx
+++ b/sw/inc/undobj.hxx
@@ -135,12 +135,12 @@ enum class DelContentType : sal_uInt16
     Fly          = 0x02,
     Bkm          = 0x08,
     AllMask      = 0x0b,
+    WriterfilterHack = 0x20,
     ExcludeFlyAtStartEnd = 0x40,
     CheckNoCntnt = 0x80,
-    CopyText = 0x100,
 };
 namespace o3tl {
-    template<> struct typed_flags<DelContentType> : is_typed_flags<DelContentType, 0x1cb> {};
+    template<> struct typed_flags<DelContentType> : is_typed_flags<DelContentType, 0xeb> {};
 }
 
 /// will DelContentIndex destroy a frame anchored at character at rAnchorPos?
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index af698f237a5b..c10cd56e93d5 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -1831,16 +1831,22 @@ DocumentContentOperationsManager::DocumentContentOperationsManager( SwDoc& i_rSw
 /**
  * Checks if rStart..rEnd mark a range that makes sense to copy.
  *
- * bCopyText means that an empty range is OK, since paragraph-anchored objects may present.
+ * bCopyText is misnamed and means that the copy is a move to create a fly
+ * and so existing flys at the edge must not be copied.
  */
 static bool IsEmptyRange(const SwPosition& rStart, const SwPosition& rEnd, bool bCopyText)
 {
-    bool bEmptyRange = rStart >= rEnd;
-    if (bCopyText)
+    if (rStart == rEnd)
+    {   // check if a fly anchored there would be copied - then copy...
+        return !IsDestroyFrameAnchoredAtChar(rStart, rStart, rEnd,
+                bCopyText
+                    ? DelContentType::WriterfilterHack|DelContentType::AllMask
+                    : DelContentType::AllMask);
+    }
+    else
     {
-        bEmptyRange = rStart > rEnd;
+        return rEnd < rStart;
     }
-    return bEmptyRange;
 }
 
 // Copy an area into this document or into another document
@@ -3502,24 +3508,22 @@ void DocumentContentOperationsManager::CopyFlyInFlyImpl(
             break;
             case RndStdIds::FLY_AT_PARA:
                 {
-                    DelContentType eDelContentType = DelContentType::AllMask;
-                    if (bCopyText)
-                    {
-                        // Called from SwXText::copyText(), select the frame if rAnchorPos is inside
-                        // the range, inclusive.
-                        eDelContentType = DelContentType::CopyText;
-                    }
                     bAdd = IsSelectFrameAnchoredAtPara(*pAPos,
                         pCopiedPaM ? *pCopiedPaM->Start() : SwPosition(rRg.aStart),
                         pCopiedPaM ? *pCopiedPaM->End() : SwPosition(rRg.aEnd),
-                        eDelContentType);
+                        bCopyText
+                            ? DelContentType::AllMask|DelContentType::WriterfilterHack
+                            : DelContentType::AllMask);
                 }
             break;
             case RndStdIds::FLY_AT_CHAR:
                 {
                     bAdd = IsDestroyFrameAnchoredAtChar(*pAPos,
                         pCopiedPaM ? *pCopiedPaM->Start() : SwPosition(rRg.aStart),
-                        pCopiedPaM ? *pCopiedPaM->End() : SwPosition(rRg.aEnd));
+                        pCopiedPaM ? *pCopiedPaM->End() : SwPosition(rRg.aEnd),
+                        bCopyText
+                            ? DelContentType::AllMask|DelContentType::WriterfilterHack
+                            : DelContentType::AllMask);
                 }
             break;
             default:
diff --git a/sw/source/core/doc/docedt.cxx b/sw/source/core/doc/docedt.cxx
index c9e2aad31551..192738317a05 100644
--- a/sw/source/core/doc/docedt.cxx
+++ b/sw/source/core/doc/docedt.cxx
@@ -221,12 +221,12 @@ void DelFlyInRange( const SwNodeIndex& rMkNdIdx,
         if (pAPos &&
             (((rAnch.GetAnchorId() == RndStdIds::FLY_AT_PARA)
                 && IsSelectFrameAnchoredAtPara(*pAPos, rStart, rEnd, pPtIdx
-                    ? DelContentType::AllMask
-                    : DelContentType::AllMask|DelContentType::CheckNoCntnt))
+                    ? DelContentType::AllMask|DelContentType::WriterfilterHack
+                    : DelContentType::AllMask|DelContentType::WriterfilterHack|DelContentType::CheckNoCntnt))
             || ((rAnch.GetAnchorId() == RndStdIds::FLY_AT_CHAR)
                 && IsDestroyFrameAnchoredAtChar(*pAPos, rStart, rEnd, pPtIdx
-                    ? DelContentType::AllMask
-                    : DelContentType::AllMask|DelContentType::CheckNoCntnt))))
+                    ? DelContentType::AllMask|DelContentType::WriterfilterHack
+                    : DelContentType::AllMask|DelContentType::WriterfilterHack|DelContentType::CheckNoCntnt))))
         {
                 // If the Fly is deleted, all Flys in its content have to be deleted too.
                 const SwFormatContent &rContent = pFormat->GetContent();
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 8feb5986d509..2a63e2112b6b 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -428,7 +428,7 @@ SwFlyFrameFormat* SwDoc::MakeFlyAndMove( const SwPaM& rPam, const SfxItemSet& rS
                         *rTmp.GetPoint() != *rTmp.GetMark() )
                     {
                         // aPos is the newly created fly section, so definitely outside rPam, it's pointless to check that again.
-                        getIDocumentContentOperations().CopyRange( *const_cast<SwPaM*>(&rTmp), aPos, /*bCopyAll=*/false, /*bCheckPos=*/false, /*bCopyText=*/false );
+                        getIDocumentContentOperations().CopyRange( *const_cast<SwPaM*>(&rTmp), aPos, /*bCopyAll=*/false, /*bCheckPos=*/false, /*bCopyText=*/true);
                     }
                 }
                 getIDocumentRedlineAccess().SetRedlineMove(bOldRedlineMove);
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index 08f6f466f318..378df9595e50 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -1554,8 +1554,9 @@ bool IsDestroyFrameAnchoredAtChar(SwPosition const & rAnchorPos,
             && (rStart.nNode <= rAnchorPos.nNode);
     }
 
-    if (rAnchorPos.GetDoc()->IsInReading())
-    {   // FIXME hack for writerfilter RemoveLastParagraph(); can't test file format more specific?
+    if ((nDelContentType & DelContentType::WriterfilterHack)
+        && rAnchorPos.GetDoc()->IsInReading())
+    {   // FIXME hack for writerfilter RemoveLastParagraph() and MakeFlyAndMove(); can't test file format more specific?
         return (rStart < rAnchorPos) && (rAnchorPos < rEnd);
     }
 
@@ -1590,13 +1591,9 @@ bool IsSelectFrameAnchoredAtPara(SwPosition const & rAnchorPos,
             && (rStart.nNode <= rAnchorPos.nNode);
     }
 
-    if (nDelContentType == DelContentType::CopyText)
-    {
-        return (rStart.nNode <= rAnchorPos.nNode) && (rAnchorPos.nNode <= rEnd.nNode);
-    }
-
-    if (rAnchorPos.GetDoc()->IsInReading())
-    {   // FIXME hack for writerfilter RemoveLastParagraph(); can't test file format more specific?
+    if ((nDelContentType & DelContentType::WriterfilterHack)
+        && rAnchorPos.GetDoc()->IsInReading())
+    {   // FIXME hack for writerfilter RemoveLastParagraph() and MakeFlyAndMove(); can't test file format more specific?
         // but it MUST NOT be done during the SetRedlineFlags at the end of ODF
         // import, where the IsInXMLImport() cannot be checked because the
         // stupid code temp. overrides it - instead rely on setting the ALLFLYS
diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx
index 71894e2d1908..db748899c582 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -2254,7 +2254,7 @@ SwXText::copyText(
             // Explicitly request copy text mode, so
             // sw::DocumentContentOperationsManager::CopyFlyInFlyImpl() will copy shapes anchored to
             // us, even if we have only a single paragraph.
-            m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(temp, rPos, /*bCopyAll=*/false, /*bCheckPos=*/true, /*bCopyText=*/true);
+            m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(temp, rPos, /*bCopyAll=*/false, /*bCheckPos=*/true, /*bCopyText=*/false);
         }
         if (!pFirstNode)
         {   // the node at rPos was split; get rid of the first empty one so
@@ -2265,7 +2265,7 @@ SwXText::copyText(
     }
     else
     {
-        m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(*pCursor->GetPaM(), rPos, /*bCopyAll=*/false, /*bCheckPos=*/true, /*bCopyText=*/true);
+        m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(*pCursor->GetPaM(), rPos, /*bCopyAll=*/false, /*bCheckPos=*/true, /*bCopyText=*/false);
     }
 
 }