sw/inc/IDocumentContentOperations.hxx | 3 +-
sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx |binary
sw/qa/core/doc/doc.cxx | 16 +++++++++++
sw/source/core/doc/DocumentContentOperationsManager.cxx | 16 ++++++++---
sw/source/core/doc/docdesc.cxx | 4 +-
sw/source/core/doc/docfmt.cxx | 2 -
sw/source/core/inc/DocumentContentOperationsManager.hxx | 2 -
sw/source/core/unocore/unotext.cxx | 22 +++++++++++++---
writerfilter/source/dmapper/PropertyMap.cxx | 11 ++++++++
9 files changed, 64 insertions(+), 12 deletions(-)
New commits:
commit 3ec224dcb15e0e663ba85077b8ea0e632f8f03f8
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Sep 8 16:41:46 2022 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Sep 8 17:46:05 2022 +0200
DOCX import: avoid duplicated bookmarks in copied header/footer text
In case a DOCX document has 2 sections and the 2nd section has a linked
header, writerfilter/ copies the header text from the 1st page style to
the 2nd page style. This leads to duplicated bookmarks in the doc model,
which causes an unexpected increase in bookmark count on export.
This went wrong with 8885bdf2f564bb7b56181c50baf39ff99d551ec9
(tdf#128106 sw: copy bookmarks at start if whole node is copied,
2021-08-27), previously at least bookmarks at the start of the header
were not duplicated, though that was also inconsistent, since other
bookmarks were still duplicated. Interestingly the DOC import doesn't
have this problem, because it first copies header text and only later
imports the bookmarks.
Fix the problem by introducing a new SwCopyFlags::SkipBookmarks flag,
add UNO API to set this from writerfilter/ code and skip copying
bookmarks (but not other marks) in sw::CopyBookmarks() accordingly.
Copying other marks is still needed, because e.g. fieldmarks have a mark
and a field, and it would be inconsistent to copy the field, but not the
mark.
Note that the text is still duplicated in header/footer, linked
header/footer is a missing feature on the Writer side.
Change-Id: I40e18f231ef2c0d91ae9582621684ef5b6284904
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139697
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/sw/inc/IDocumentContentOperations.hxx
b/sw/inc/IDocumentContentOperations.hxx
index 5a95d0ba95b8..60d45d47a0a3 100644
--- a/sw/inc/IDocumentContentOperations.hxx
+++ b/sw/inc/IDocumentContentOperations.hxx
@@ -75,11 +75,12 @@ enum class SwCopyFlags
CopyAll = (1<<0), ///< copy break attributes even when source is
single node
CheckPosInFly = (1<<1), ///< check if target position is in fly anchored
at source range
IsMoveToFly = (1<<2), ///< MakeFlyAndMove
+ SkipBookmarks = (1<<3), ///< skip bookmarks, but not other kind of marks
// TODO: mbCopyIsMove? mbIsRedlineMove?
};
namespace o3tl
{
- template<> struct typed_flags<SwCopyFlags> : is_typed_flags<SwCopyFlags,
0x07> {};
+ template<> struct typed_flags<SwCopyFlags> : is_typed_flags<SwCopyFlags,
0x0f> {};
}
enum class SwDeleteFlags
diff --git a/sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx
b/sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx
new file mode 100644
index 000000000000..3fb27b430a17
Binary files /dev/null and b/sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx
differ
diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx
index 87bfd2697621..2ee2ee342bd3 100644
--- a/sw/qa/core/doc/doc.cxx
+++ b/sw/qa/core/doc/doc.cxx
@@ -255,6 +255,22 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest,
testContentControlDelete)
CPPUNIT_ASSERT_EQUAL(OUString("\x0001test\x0001"), pTextNode->GetText());
}
+CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testCopyFlagSkipBookmarks)
+{
+ // Given a document with a bookmark in a header that is linked later:
+ SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "copy-flag-skip-bookmarks.docx");
+
+ // When checking the # of bookmarks in the resulting doc model:
+ sal_Int32 nActual = pDoc->getIDocumentMarkAccess()->getAllMarksCount();
+
+ // Then make sure we have a single bookmark, with no duplications:
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 1
+ // - Actual : 2
+ // i.e. the 2nd header had a duplicated bookmark.
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), nActual);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx
b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index c642cde1c8c7..62c362eba181 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -232,7 +232,7 @@ namespace
namespace sw
{
// TODO: use SaveBookmark (from DelBookmarks)
- void CopyBookmarks(const SwPaM& rPam, const SwPosition& rCpyPam)
+ void CopyBookmarks(const SwPaM& rPam, const SwPosition& rCpyPam,
SwCopyFlags eFlags)
{
const SwDoc& rSrcDoc = rPam.GetDoc();
SwDoc& rDestDoc = rCpyPam.GetDoc();
@@ -283,6 +283,7 @@ namespace sw
// We have to count the "non-copied" nodes...
SwNodeOffset nDelCount;
SwNodeIndex aCorrIdx(InitDelCount(rPam, nDelCount));
+ auto bSkipBookmarks = static_cast<bool>(eFlags &
SwCopyFlags::SkipBookmarks);
for(const sw::mark::IMark* const pMark : vMarksToCopy)
{
SwPaM aTmpPam(*pCpyStt);
@@ -295,10 +296,17 @@ namespace sw
lcl_SetCpyPos(pMark->GetOtherMarkPos(), rStt, *pCpyStt,
*aTmpPam.GetMark(), nDelCount);
}
+ IDocumentMarkAccess::MarkType eType =
IDocumentMarkAccess::GetType(*pMark);
+ if (bSkipBookmarks && eType ==
IDocumentMarkAccess::MarkType::BOOKMARK)
+ {
+ // It was requested to skip bookmarks while copying. Do that,
but continue to copy
+ // other kind of marks, like fieldmarks.
+ continue;
+ }
::sw::mark::IMark* const pNewMark =
rDestDoc.getIDocumentMarkAccess()->makeMark(
aTmpPam,
pMark->GetName(),
- IDocumentMarkAccess::GetType(*pMark),
+ eType,
::sw::mark::InsertMode::CopyText);
// Explicitly try to get exactly the same name as in the source
// because NavigatorReminders, DdeBookmarks etc. ignore the
proposed name
@@ -3672,7 +3680,7 @@ void DocumentContentOperationsManager::CopyWithFlyInFly(
targetPos = pCopiedPaM->second;
}
- sw::CopyBookmarks(pCopiedPaM ? pCopiedPaM->first : aRgTmp, targetPos);
+ sw::CopyBookmarks(pCopiedPaM ? pCopiedPaM->first : aRgTmp, targetPos,
flags);
}
if (rRg.aStart != rRg.aEnd)
@@ -5301,7 +5309,7 @@ bool
DocumentContentOperationsManager::CopyImplImpl(SwPaM& rPam, SwPosition& rPo
// Also copy all bookmarks
if( bCopyBookmarks && m_rDoc.getIDocumentMarkAccess()->getAllMarksCount() )
{
- sw::CopyBookmarks(rPam, *pCopyPam->Start());
+ sw::CopyBookmarks(rPam, *pCopyPam->Start(), SwCopyFlags::Default);
}
if( RedlineFlags::DeleteRedlines & eOld )
diff --git a/sw/source/core/doc/docdesc.cxx b/sw/source/core/doc/docdesc.cxx
index d75caf8cac0b..a5642f053b7c 100644
--- a/sw/source/core/doc/docdesc.cxx
+++ b/sw/source/core/doc/docdesc.cxx
@@ -302,7 +302,7 @@ void SwDoc::CopyMasterHeader(const SwPageDesc &rChged,
const SwFormatHeader &rHe
GetDocumentContentOperationsManager().CopyFlyInFlyImpl(aRange, nullptr,
*pSttNd);
SwPaM const source(aRange.aStart, aRange.aEnd);
SwPosition dest(*pSttNd);
- sw::CopyBookmarks(source, dest);
+ sw::CopyBookmarks(source, dest, SwCopyFlags::Default);
pFormat->SetFormatAttr( SwFormatContent( pSttNd ) );
rDescFrameFormat.SetFormatAttr( SwFormatHeader( pFormat )
);
}
@@ -379,7 +379,7 @@ void SwDoc::CopyMasterFooter(const SwPageDesc &rChged,
const SwFormatFooter &rFo
GetDocumentContentOperationsManager().CopyFlyInFlyImpl(aRange, nullptr,
*pSttNd);
SwPaM const source(aRange.aStart, aRange.aEnd);
SwPosition dest(*pSttNd);
- sw::CopyBookmarks(source, dest);
+ sw::CopyBookmarks(source, dest, SwCopyFlags::Default);
pFormat->SetFormatAttr( SwFormatContent( pSttNd ) );
rDescFrameFormat.SetFormatAttr( SwFormatFooter( pFormat )
);
}
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index 357ca66aeb8d..51c0e935e7b2 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -1406,7 +1406,7 @@ void SwDoc::CopyPageDescHeaderFooterImpl( bool bCpyHeader,
// TODO: investigate calling CopyWithFlyInFly?
SwPaM const source(aRg.aStart, aRg.aEnd);
SwPosition dest(*pSttNd);
- sw::CopyBookmarks(source, dest);
+ sw::CopyBookmarks(source, dest, SwCopyFlags::Default);
pNewFormat->SetFormatAttr( SwFormatContent( pSttNd ));
}
else
diff --git a/sw/source/core/inc/DocumentContentOperationsManager.hxx
b/sw/source/core/inc/DocumentContentOperationsManager.hxx
index 49473930f7fe..d2b61500c8c6 100644
--- a/sw/source/core/inc/DocumentContentOperationsManager.hxx
+++ b/sw/source/core/inc/DocumentContentOperationsManager.hxx
@@ -182,7 +182,7 @@ private:
};
-void CopyBookmarks(const SwPaM& rPam, const SwPosition& rTarget);
+void CopyBookmarks(const SwPaM& rPam, const SwPosition& rTarget, SwCopyFlags
eFlags);
void CalcBreaks(std::vector<std::pair<SwNodeOffset, sal_Int32>> & rBreaks,
SwPaM const & rPam, bool const isOnlyFieldmarks = false);
diff --git a/sw/source/core/unocore/unotext.cxx
b/sw/source/core/unocore/unotext.cxx
index c68a1a331c90..5d65f5ef8b63 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -86,6 +86,7 @@ public:
const CursorType m_eType;
SwDoc * m_pDoc;
bool m_bIsValid;
+ bool m_bCopySkipsBookmarks;
Impl( SwXText & rThis,
SwDoc *const pDoc, const CursorType eType)
@@ -94,6 +95,7 @@ public:
, m_eType(eType)
, m_pDoc(pDoc)
, m_bIsValid(nullptr != pDoc)
+ , m_bCopySkipsBookmarks(false)
{
}
@@ -1121,9 +1123,18 @@ SwXText::getPropertySetInfo()
}
void SAL_CALL
-SwXText::setPropertyValue(const OUString& /*aPropertyName*/,
- const uno::Any& /*aValue*/)
+SwXText::setPropertyValue(const OUString& aPropertyName,
+ const uno::Any& aValue)
{
+ if (aPropertyName == "CopySkipsBookmarks")
+ {
+ bool bValue{};
+ if (aValue >>= bValue)
+ {
+ m_pImpl->m_bCopySkipsBookmarks = bValue;
+ }
+ return;
+ }
throw lang::IllegalArgumentException();
}
@@ -2402,7 +2413,12 @@ 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, SwCopyFlags::CheckPosInFly);
+ SwCopyFlags eFlags = SwCopyFlags::CheckPosInFly;
+ if (m_pImpl->m_bCopySkipsBookmarks)
+ {
+ eFlags |= SwCopyFlags::SkipBookmarks;
+ }
+ m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(temp,
rPos, eFlags);
}
if (!pFirstNode)
{ // the node at rPos was split; get rid of the first empty one so
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx
b/writerfilter/source/dmapper/PropertyMap.cxx
index eec9b678a209..e848a5780ebf 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -874,7 +874,18 @@ void SectionPropertyMap::CopyHeaderFooterTextProperty(
const uno::Reference< bea
if ( xPrevStyle.is() )
xPrevTxt.set( xPrevStyle->getPropertyValue( sName ),
uno::UNO_QUERY_THROW );
+ uno::Reference<beans::XPropertySet> xTxtProps(xTxt, uno::UNO_QUERY);
+ if (xTxtProps.is())
+ {
+ // Skip copying bookmarks, so the number of bookmarks in the
source document and in the
+ // resulting doc model match.
+ xTxtProps->setPropertyValue("CopySkipsBookmarks", uno::Any(true));
+ }
xTxt->copyText( xPrevTxt );
+ if (xTxtProps.is())
+ {
+ xTxtProps->setPropertyValue("CopySkipsBookmarks", uno::Any(false));
+ }
}
catch ( const uno::Exception& )
{
