fdo#85542: fix DOCX import of overlapping bookmarks

This was broken by 345a3a394e082595924bf219796627f6c00ae2dd.
Kill the static variable and instead have some more state in the
implementation. Still not ideal, but at least fixes the regression.

Change-Id: I562f7d88a1983bd0ec2e01d6bb1e4a53551d0953
Reviewed-on: https://gerrit.libreoffice.org/12491
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos@collabora.co.uk>

5 files changed, 60 insertions, 14 deletions

diff --git a/sw/qa/extras/ooxmlimport/data/fdo85542.docx b/sw/qa/extras/ooxmlimport/data/fdo85542.docx
new file mode 100644
index 000000000000..db4940845140
--- /dev/null
+++ b/sw/qa/extras/ooxmlimport/data/fdo85542.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 57ae73534127..3f2dd6baad2b 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -32,6 +32,7 @@
 #include <com/sun/star/text/VertOrientation.hpp>
 #include <com/sun/star/text/WrapTextMode.hpp>
 #include <com/sun/star/text/WritingMode2.hpp>
+#include <com/sun/star/text/XBookmarksSupplier.hpp>
 #include <com/sun/star/text/XDependentTextField.hpp>
 #include <com/sun/star/text/XFormField.hpp>
 #include <com/sun/star/text/XPageCursor.hpp>
@@ -2501,6 +2502,36 @@ DECLARE_OOXMLIMPORT_TEST(testBnc821804, "bnc821804.docx")
     CPPUNIT_ASSERT_EQUAL(false,getProperty<bool>(getRun(getParagraph(10), 3), "IsStart"));
 }
 
+DECLARE_OOXMLIMPORT_TEST(testFdo85542, "fdo85542.docx")
+{
+    //CPPUNIT_ASSERT_EQUAL(false,true);
+    uno::Reference<text::XBookmarksSupplier> xBookmarksSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xBookmarksByIdx(xBookmarksSupplier->getBookmarks(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(xBookmarksByIdx->getCount(), 3);
+    uno::Reference<container::XNameAccess> xBookmarksByName(xBookmarksSupplier->getBookmarks(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xBookmarksByName->hasByName("B1"));
+    CPPUNIT_ASSERT(xBookmarksByName->hasByName("B2"));
+    CPPUNIT_ASSERT(xBookmarksByName->hasByName("B3"));
+    // B1
+    uno::Reference<text::XTextContent> xContent1(xBookmarksByName->getByName("B1"), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xRange1(xContent1->getAnchor(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(xRange1->getString(), OUString("ABB"));
+    // B2
+    uno::Reference<text::XTextContent> xContent2(xBookmarksByName->getByName("B2"), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xRange2(xContent2->getAnchor(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(xRange2->getString(), OUString("BBC"));
+    // B3 -- testing a collapsed bookmark
+    uno::Reference<text::XTextContent> xContent3(xBookmarksByName->getByName("B3"), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xRange3(xContent3->getAnchor(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(xRange3->getString(), OUString(""));
+    uno::Reference<text::XText> xText(xRange3->getText( ), uno::UNO_QUERY);
+    uno::Reference<text::XTextCursor> xNeighborhoodCursor(xText->createTextCursor( ), uno::UNO_QUERY);
+    xNeighborhoodCursor->gotoRange(xRange3, false);
+    xNeighborhoodCursor->goLeft(1, false);
+    xNeighborhoodCursor->goRight(2, true);
+    uno::Reference<text::XTextRange> xTextNeighborhood(xNeighborhoodCursor, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(xTextNeighborhood->getString(), OUString("AB"));
+}
 #endif
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 5b00fd6ad43d..8d4f548e2a54 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -183,7 +183,6 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
     if (m_pImpl->getTableManager().attribute(nName, val))
         return;
 
-    static OUString sLocalBookmarkName;
     static const int nSingleLineSpacing = 240;
     sal_Int32 nIntValue = val.getInt();
     OUString sStringValue = val.getString();
@@ -220,14 +219,15 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
         case NS_ooxml::LN_endnote:
             break;
         case NS_ooxml::LN_CT_Bookmark_name:
-            // sStringValue contains the bookmark name
-            sLocalBookmarkName = sStringValue;
+            // SAL_DEBUG("LN_CT_Bookmark_name " << sStringValue);
+            m_pImpl->SetBookmarkName( sStringValue );
         break;
         case NS_ooxml::LN_CT_MarkupRangeBookmark_id:
-            //contains the bookmark identifier - has to be added to the bookmark name imported before
-            //if it is already available then the bookmark should be inserted
-            m_pImpl->AddBookmark( sLocalBookmarkName, sStringValue );
-            sLocalBookmarkName.clear();
+            // SAL_DEBUG("LN_CT_MarkupRangeBookmark_id " << sStringValue);
+            // add a bookmark range -- this remembers a bookmark starting here
+            // or, if the bookmark was already started or, if the bookmark was
+            // already started before, writes out the bookmark
+            m_pImpl->StartOrEndBookmark( sStringValue );
         break;
         case NS_ooxml::LN_CT_MarkupRange_displacedByCustomXml:
             break;
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index c6b8565a9be9..b4fa15bd45e4 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -4341,8 +4341,19 @@ void DomainMapper_Impl::PopFieldContext()
 }
 
 
-void DomainMapper_Impl::AddBookmark( const OUString& rBookmarkName, const OUString& rId )
+void DomainMapper_Impl::SetBookmarkName( const OUString& rBookmarkName )
 {
+    // SAL_DEBUG("DomainMapper_Impl::SetBookmarkName for id " << m_sCurrentBkmkId << " to " << rBookmarkName);
+    BookmarkMap_t::iterator aBookmarkIter = m_aBookmarkMap.find( m_sCurrentBkmkId );
+    if( aBookmarkIter != m_aBookmarkMap.end() )
+        aBookmarkIter->second.m_sBookmarkName = rBookmarkName;
+    else
+        m_sCurrentBkmkName = rBookmarkName;
+}
+
+void DomainMapper_Impl::StartOrEndBookmark( const OUString& rId )
+{
+    // SAL_DEBUG("DomainMapper_Impl::AddBookmark " << rId);
     /*
      * Add the dummy paragraph to handle section properties
      * iff the first element in the section is a table. If the dummy para is not added yet, then add it;
@@ -4388,14 +4399,13 @@ void DomainMapper_Impl::AddBookmark( const OUString& rBookmarkName, const OUStri
                     xCursor->goLeft( 1, false );
                 }
                 uno::Reference< container::XNamed > xBkmNamed( xBookmark, uno::UNO_QUERY_THROW );
+                assert(!aBookmarkIter->second.m_sBookmarkName.isEmpty());
                 //todo: make sure the name is not used already!
-                if ( !aBookmarkIter->second.m_sBookmarkName.isEmpty() )
-                    xBkmNamed->setName( aBookmarkIter->second.m_sBookmarkName );
-                else
-                    xBkmNamed->setName( rBookmarkName );
+                xBkmNamed->setName( aBookmarkIter->second.m_sBookmarkName );
                 xTextAppend->insertTextContent( uno::Reference< text::XTextRange >( xCursor, uno::UNO_QUERY_THROW), xBookmark, !xCursor->isCollapsed() );
             }
             m_aBookmarkMap.erase( aBookmarkIter );
+            m_sCurrentBkmkId.clear();
         }
         else
         {
@@ -4410,7 +4420,9 @@ void DomainMapper_Impl::AddBookmark( const OUString& rBookmarkName, const OUStri
                     bIsStart = !xCursor->goLeft(1, false);
                 xCurrent = xCursor->getStart();
             }
-            m_aBookmarkMap.insert(BookmarkMap_t::value_type( rId, BookmarkInsertPosition( bIsStart, rBookmarkName, xCurrent ) ));
+            m_sCurrentBkmkId = rId;
+            m_aBookmarkMap.insert(BookmarkMap_t::value_type( rId, BookmarkInsertPosition( bIsStart, m_sCurrentBkmkName, xCurrent ) ));
+            m_sCurrentBkmkName.clear();
         }
     }
     catch( const uno::Exception& )
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 69137f2cd813..8daf26ea1781 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -351,6 +351,8 @@ private:
     LineNumberSettings                                                              m_aLineNumberSettings;
 
     BookmarkMap_t                                                                   m_aBookmarkMap;
+    OUString                                                                        m_sCurrentBkmkId;
+    OUString                                                                        m_sCurrentBkmkName;
 
     _PageMar                                                                        m_aPageMargins;
     sal_Int32                                                                       m_nSymboldata;
@@ -667,7 +669,8 @@ public:
     //the end of field is reached (0x15 appeared) - the command might still be open
     void PopFieldContext();
 
-    void AddBookmark( const OUString& rBookmarkName, const OUString& rId );
+    void SetBookmarkName( const OUString& rBookmarkName );
+    void StartOrEndBookmark( const OUString& rId );
 
     void AddAnnotationPosition(
         const bool bStart,