sw/inc/undobj.hxx                                       |    4 +-
 sw/source/core/doc/DocumentContentOperationsManager.cxx |   32 +++++++++-------
 sw/source/core/doc/docedt.cxx                           |    8 ++--
 sw/source/core/doc/doclay.cxx                           |    2 -
 sw/source/core/undo/undobj.cxx                          |   15 +++----
 sw/source/core/unocore/unotext.cxx                      |    4 +-
 6 files changed, 33 insertions(+), 32 deletions(-)

New commits:
commit 81ec0039b2085faab49380c7a56af0c562d4c9e4
Author:     Michael Stahl <michael.st...@cib.de>
AuthorDate: Mon Jan 20 13:48:27 2020 +0100
Commit:     Michael Stahl <michael.st...@cib.de>
CommitDate: Mon Jan 20 19:25:41 2020 +0100

    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.st...@cib.de>

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index 5f261f68e8eb..4b9b0f48ee51 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 0cc15a92e4d2..7c880c26ff68 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 28dfb4990468..b7dcdf724677 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 ee431f5955af..b5a98cf724dd 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 5831f8bfea89..9aabda3b26ca 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -1556,8 +1556,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);
     }
 
@@ -1592,13 +1593,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 0c98cae57a3e..2857b2808cca 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -2252,7 +2252,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
@@ -2263,7 +2263,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);
     }
 
 }
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to