sw/inc/IDocumentContentOperations.hxx | 3 --
sw/inc/strings.hrc | 2 +
sw/qa/core/doc/doc.cxx | 24 ++++++++++------
sw/source/core/doc/DocumentContentOperationsManager.cxx | 16 ++--------
sw/source/core/doc/docbm.cxx | 4 ++
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 -------
10 files changed, 33 insertions(+), 57 deletions(-)
New commits:
commit 41403fbff8140ad0ca7cf8f81d37cddcfbd19197
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Oct 13 13:53:33 2022 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Oct 13 15:16:04 2022 +0200
sw: improve duplicated bookmarks in copied header/footer text
Bookmarks in copied text were renamed in a way that made it hard to
differentiate between the original bookmark and the copy, e.g. "Bookmark
1" was renamed to "Bookmark 11".
Bookmarks are supposed to have a unique name, so renaming makes sense,
and it's probably better to do that compared to what commit
3ec224dcb15e0e663ba85077b8ea0e632f8f03f8 (DOCX import: avoid duplicated
bookmarks in copied header/footer text, 2022-09-08) did to just omit
them during copy. That solved the duplicated bookmarks, but if one had
bookmarks around images to find them, now it doesn't work to find all
such images.
Fix the problem by going back to copying bookmarks, but copy "Bookmark
1" as "Bookmark 1 Copy 1" (and "Bookmark 1 Copy 2", etc), so API users
can identify the original and the copied bookmarks.
A similar problem is there for images as well, but that's not yet
addressed here.
Change-Id: Ic0689b05f790a99ff06523ff4836b1dd412af896
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141293
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/sw/inc/IDocumentContentOperations.hxx
b/sw/inc/IDocumentContentOperations.hxx
index 60d45d47a0a3..5a95d0ba95b8 100644
--- a/sw/inc/IDocumentContentOperations.hxx
+++ b/sw/inc/IDocumentContentOperations.hxx
@@ -75,12 +75,11 @@ 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,
0x0f> {};
+ template<> struct typed_flags<SwCopyFlags> : is_typed_flags<SwCopyFlags,
0x07> {};
}
enum class SwDeleteFlags
diff --git a/sw/inc/strings.hrc b/sw/inc/strings.hrc
index 4f2dd39ff86b..e1859f1432d0 100644
--- a/sw/inc/strings.hrc
+++ b/sw/inc/strings.hrc
@@ -1446,6 +1446,8 @@
// in order to change %PRODUCTNAME at runtime is expensive, so limit doing
that as much as possible.
#define STR_A11Y_DESC_AUTO
NC_("insertcaption|extended_tip|auto", "Opens the Caption dialog. It has the
same information as the dialog you get by menu %PRODUCTNAME Writer -
AutoCaption in the Options dialog box.")
+#define STR_MARK_COPY NC_("STR_MARK_COPY", "%1 Copy ")
+
#endif
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx
b/sw/qa/core/doc/data/copy-bookmarks.docx
similarity index 100%
rename from sw/qa/core/doc/data/copy-flag-skip-bookmarks.docx
rename to sw/qa/core/doc/data/copy-bookmarks.docx
diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx
index 8c882e906373..98548befa9ee 100644
--- a/sw/qa/core/doc/doc.cxx
+++ b/sw/qa/core/doc/doc.cxx
@@ -256,19 +256,27 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest,
testContentControlDelete)
CPPUNIT_ASSERT_EQUAL(OUString("\x0001test\x0001"), pTextNode->GetText());
}
-CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testCopyFlagSkipBookmarks)
+CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testCopyBookmarks)
{
// 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:
+ SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "copy-bookmarks.docx");
+
+ // When checking the # of non-copy bookmarks in the resulting doc model:
+ sal_Int32 nActual = 0;
+ for (auto it = pDoc->getIDocumentMarkAccess()->getBookmarksBegin();
+ it != pDoc->getIDocumentMarkAccess()->getBookmarksEnd(); ++it)
+ {
+ if ((*it)->GetName().indexOf("Copy") == -1)
+ {
+ ++nActual;
+ }
+ }
+
+ // Then make sure we have a single non-copy 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.
+ // i.e. the 2nd header had a duplicated bookmark without "Copy" in its
name.
CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), nActual);
}
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx
b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 51c0fdb9cba9..d7896c6c113d 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,
SwCopyFlags eFlags)
+ void CopyBookmarks(const SwPaM& rPam, const SwPosition& rCpyPam)
{
const SwDoc& rSrcDoc = rPam.GetDoc();
SwDoc& rDestDoc = rCpyPam.GetDoc();
@@ -283,7 +283,6 @@ 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);
@@ -296,17 +295,10 @@ 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(),
- eType,
+ IDocumentMarkAccess::GetType(*pMark),
::sw::mark::InsertMode::CopyText);
// Explicitly try to get exactly the same name as in the source
// because NavigatorReminders, DdeBookmarks etc. ignore the
proposed name
@@ -3676,7 +3668,7 @@ void DocumentContentOperationsManager::CopyWithFlyInFly(
targetPos = pCopiedPaM->second;
}
- sw::CopyBookmarks(pCopiedPaM ? pCopiedPaM->first : aRgTmp, targetPos,
flags);
+ sw::CopyBookmarks(pCopiedPaM ? pCopiedPaM->first : aRgTmp, targetPos);
}
if (rRg.aStart != rRg.aEnd)
@@ -5299,7 +5291,7 @@ bool
DocumentContentOperationsManager::CopyImplImpl(SwPaM& rPam, SwPosition& rPo
// Also copy all bookmarks
if( bCopyBookmarks && m_rDoc.getIDocumentMarkAccess()->getAllMarksCount() )
{
- sw::CopyBookmarks(rPam, *pCopyPam->Start(), SwCopyFlags::Default);
+ sw::CopyBookmarks(rPam, *pCopyPam->Start());
}
if( RedlineFlags::DeleteRedlines & eOld )
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 76a684282e7c..d38542922b52 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -48,6 +48,7 @@
#include <libxml/xmlstring.h>
#include <libxml/xmlwriter.h>
#include <comphelper/lok.hxx>
+#include <strings.hrc>
constexpr OUStringLiteral S_ANNOTATION_BOOKMARK = u"____";
@@ -1717,9 +1718,10 @@ namespace sw::mark
sal_Int32 nCnt = 1;
MarkBasenameMapUniqueOffset_t::const_iterator aIter =
m_aMarkBasenameMapUniqueOffset.find(rName);
if(aIter != m_aMarkBasenameMapUniqueOffset.end()) nCnt = aIter->second;
+ OUString aPrefix = SwResId(STR_MARK_COPY).replaceFirst("%1", rName);
while(nCnt < SAL_MAX_INT32)
{
- sTmp = rName + OUString::number(nCnt);
+ sTmp = aPrefix + OUString::number(nCnt);
nCnt++;
if (lcl_FindMarkByName(sTmp, m_vAllMarks.begin(),
m_vAllMarks.end()) == m_vAllMarks.end())
{
diff --git a/sw/source/core/doc/docdesc.cxx b/sw/source/core/doc/docdesc.cxx
index a5642f053b7c..d75caf8cac0b 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, SwCopyFlags::Default);
+ sw::CopyBookmarks(source, dest);
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, SwCopyFlags::Default);
+ sw::CopyBookmarks(source, dest);
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 ff8d5b0100f7..d9eef9557cb4 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, SwCopyFlags::Default);
+ sw::CopyBookmarks(source, dest);
pNewFormat->SetFormatAttr( SwFormatContent( pSttNd ));
}
else
diff --git a/sw/source/core/inc/DocumentContentOperationsManager.hxx
b/sw/source/core/inc/DocumentContentOperationsManager.hxx
index d2b61500c8c6..49473930f7fe 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, SwCopyFlags
eFlags);
+void CopyBookmarks(const SwPaM& rPam, const SwPosition& rTarget);
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 9378016997a6..be1f76fe5353 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -86,7 +86,6 @@ public:
const CursorType m_eType;
SwDoc * m_pDoc;
bool m_bIsValid;
- bool m_bCopySkipsBookmarks;
Impl( SwXText & rThis,
SwDoc *const pDoc, const CursorType eType)
@@ -95,7 +94,6 @@ public:
, m_eType(eType)
, m_pDoc(pDoc)
, m_bIsValid(nullptr != pDoc)
- , m_bCopySkipsBookmarks(false)
{
}
@@ -1123,18 +1121,9 @@ 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();
}
@@ -2413,12 +2402,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.
- SwCopyFlags eFlags = SwCopyFlags::CheckPosInFly;
- if (m_pImpl->m_bCopySkipsBookmarks)
- {
- eFlags |= SwCopyFlags::SkipBookmarks;
- }
- m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(temp,
rPos, eFlags);
+ m_pImpl->m_pDoc->getIDocumentContentOperations().CopyRange(temp,
rPos, SwCopyFlags::CheckPosInFly);
}
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 e848a5780ebf..eec9b678a209 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -874,18 +874,7 @@ 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& )
{
