[Libreoffice-commits] core.git: sw/inc sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Mon Jan 20 18:26:16 UTC 2020
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.Stahl at cib.de>
AuthorDate: Mon Jan 20 13:48:27 2020 +0100
Commit: Michael Stahl <michael.stahl at 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.stahl at 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);
}
}
More information about the Libreoffice-commits
mailing list