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().

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>
(cherry picked from commit 34bdbb98846115671dcc71e34f6f2900fb638154)

The backport is limited to at-character flys because the at-para commit
e75dd1fc992f168f24d66595265a978071cdd277 will not be backported to 6.3;
lots of related conflicts were resolved.

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

6 files changed, 26 insertions, 14 deletions

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index 7e76a2bc7cea..01bcc5fa7cfb 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,
     ExcludeAtCharFlyAtStartEnd = 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 0219ef38af0f..6d5f4849fce4 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -1715,16 +1715,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
@@ -3456,13 +3462,18 @@ void DocumentContentOperationsManager::CopyFlyInFlyImpl(
                 // FIXME TODO why exclude start node, this seems very questionable and causes data loss on export
                 if(m_rDoc.getIDocumentRedlineAccess().IsRedlineMove())
                     ++nStart;
+#ifndef __clang__
                 (void) bCopyText;
+#endif
             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 fad87aec561f..ece6f0e8c8ec 100644
--- a/sw/source/core/doc/docedt.cxx
+++ b/sw/source/core/doc/docedt.cxx
@@ -248,8 +248,8 @@ void DelFlyInRange( const SwNodeIndex& rMkNdIdx,
                     : rPtNdIdx <= pAPos->nNode && pAPos->nNode < rMkNdIdx))
             || ((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))))
         {
             // Only move the Anchor??
             if ((rAnch.GetAnchorId() == RndStdIds::FLY_AT_PARA)
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 6b21a28be812..6c7e18ec06c6 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -446,7 +446,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 612ca3500bc3..536ecc6ae45c 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);
     }
 
diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx
index ce86471bace4..79b27a32b104 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -2241,7 +2241,7 @@ SwXText::copyText(
 
     SwNodeIndex rNdIndex( *GetStartNode( ), 1 );
     SwPosition rPos( rNdIndex );
-    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);
 }
 
 SwXBodyText::SwXBodyText(SwDoc *const pDoc)