tdf#96480: ODF import: eliminate duplicate cross reference heading bookmarks

7c3c3006deaaaf1bb3f2f4eeeaf11da3bcebe53c is apparently worse than it
appeared at first glance since there are numerous assumptions about
bookmarks, such as that if they were inserted successfully they may be
copied successfully, which isn't the case for duplicate cross reference
bookmarks.

So fix this differently, by eliminating the duplicates and mapping all
reference fields to refer to the surviving bookmark.

It was not possible to do this in SwXBookmark by checking the makeMark()
return as that would raise interesting problems such as it's currently
guaranteed to have 1:1 SwXBoomarks to core Marks so we can't just
connect 2 SwXBookmarks to the same core Mark, and we also can't leave
the SwXBookmark unconnected after attach.

Another alternative would be to temporarily allow inserting the
duplicate bookmarks and then eliminate them after the import, but what
is implemented now is to check from xmloff for duplicates, which is
reasonably simple.

Change-Id: I7ee4854d1c9d8bf74201089cbb7287b1bd8ee3b9
(cherry picked from commit 774fb6d2e7cf36b677e66c54278225b1256bd40f)
Reviewed-on: https://gerrit.libreoffice.org/21277
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

5 files changed, 6 insertions, 17 deletions

diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index d071f419018d..a0bdeb6a8fac 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -77,7 +77,7 @@ class IDocumentMarkAccess
         */
         virtual ::sw::mark::IMark* makeMark(const SwPaM& rPaM,
             const OUString& rProposedName,
-            MarkType eMark, bool = false) = 0;
+            MarkType eMark) = 0;
 
         virtual sw::mark::IFieldmark* makeFieldBookmark( const SwPaM& rPaM,
             const OUString& rName,
diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx
index addf9c5c0ba2..04d1d3cdfaba 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -421,9 +421,7 @@ DECLARE_ODFEXPORT_TEST(testDuplicateCrossRefHeadingBookmark, "CrossRefHeadingBoo
     uno::Reference<text::XTextContent> xBookmark1(
         xBookmarks->getByName("__RefHeading__8284_1826734303"), uno::UNO_QUERY);
     CPPUNIT_ASSERT(xBookmark1.is());
-    uno::Reference<text::XTextContent> xBookmark2(
-        xBookmarks->getByName("__RefHeading__1673_25705824"), uno::UNO_QUERY);
-    CPPUNIT_ASSERT(xBookmark2.is());
+    CPPUNIT_ASSERT_THROW(xBookmarks->getByName("__RefHeading__1673_25705824"), container::NoSuchElementException);
 
     uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
     uno::Reference<util::XRefreshable>(xTextFieldsSupplier->getTextFields(), uno::UNO_QUERY)->refresh();
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 6efca0e0da42..90f466fd3d8f 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -349,8 +349,7 @@ namespace sw { namespace mark
 
     ::sw::mark::IMark* MarkManager::makeMark(const SwPaM& rPaM,
         const OUString& rName,
-        const IDocumentMarkAccess::MarkType eType,
-        bool const isHorribleHackIgnoreDuplicates)
+        const IDocumentMarkAccess::MarkType eType)
     {
 #if 0
         {
@@ -373,8 +372,7 @@ namespace sw { namespace mark
             " - more than USHRT_MAX marks are not supported correctly");
         // There should only be one CrossRefBookmark per Textnode per Type
         if ((eType == MarkType::CROSSREF_NUMITEM_BOOKMARK || eType == MarkType::CROSSREF_HEADING_BOOKMARK)
-            && (lcl_FindMarkAtPos(m_vBookmarks, *rPaM.Start(), eType) != m_vBookmarks.end())
-            && !isHorribleHackIgnoreDuplicates)
+            && (lcl_FindMarkAtPos(m_vBookmarks, *rPaM.Start(), eType) != m_vBookmarks.end()))
         {   // this can happen via UNO API
             SAL_WARN("sw.core", "MarkManager::makeMark(..)"
                 " - refusing to create duplicate CrossRefBookmark");
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 6abeb4831196..c0f09c939cc8 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -37,7 +37,7 @@ namespace sw {
         public:
             MarkManager(/*[in/out]*/ SwDoc& rDoc);
             // IDocumentMarkAccess
-            virtual ::sw::mark::IMark* makeMark(const SwPaM& rPaM, const OUString& rName, IDocumentMarkAccess::MarkType eMark, bool = false) override;
+            virtual ::sw::mark::IMark* makeMark(const SwPaM& rPaM, const OUString& rName, IDocumentMarkAccess::MarkType eMark) override;
 
             virtual sw::mark::IFieldmark* makeFieldBookmark( const SwPaM& rPaM,
                 const OUString& rName,
diff --git a/sw/source/core/unocore/unobkm.cxx b/sw/source/core/unocore/unobkm.cxx
index dc049286e6d7..e254fff477d8 100644
--- a/sw/source/core/unocore/unobkm.cxx
+++ b/sw/source/core/unocore/unobkm.cxx
@@ -227,7 +227,6 @@ throw (lang::IllegalArgumentException, uno::RuntimeException)
     SwUnoInternalPaM aPam(*m_pImpl->m_pDoc);
     ::sw::XTextRangeToSwPaM(aPam, xTextRange);
     UnoActionContext aCont(m_pImpl->m_pDoc);
-    bool isHorribleHackIgnoreDuplicates(false);
     if (m_pImpl->m_sMarkName.isEmpty())
     {
          m_pImpl->m_sMarkName = "Bookmark";
@@ -242,16 +241,10 @@ throw (lang::IllegalArgumentException, uno::RuntimeException)
         IDocumentMarkAccess::IsLegalPaMForCrossRefHeadingBookmark( aPam ) )
     {
         eType = IDocumentMarkAccess::MarkType::CROSSREF_HEADING_BOOKMARK;
-        // tdf#94804 LO 4.2-5.0 create invalid duplicates that must be preserved
-        // note: do not check meta:generator, may be preserved by other versions
-        if (m_pImpl->m_pDoc->IsInXMLImport())
-        {
-            isHorribleHackIgnoreDuplicates = true;
-        }
     }
     m_pImpl->registerInMark(*this,
         m_pImpl->m_pDoc->getIDocumentMarkAccess()->makeMark(
-            aPam, m_pImpl->m_sMarkName, eType, isHorribleHackIgnoreDuplicates));
+            aPam, m_pImpl->m_sMarkName, eType));
     // #i81002#
     // Check, if bookmark has been created.
     // E.g., the creation of a cross-reference bookmark is suppress,