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 <vmiklos@collabora.com>
Tested-by: Jenkins

5 files changed, 35 insertions, 11 deletions

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