[Libreoffice-commits] core.git: Branch 'libreoffice-5-0' - include/xmloff sw/inc sw/qa sw/source xmloff/source

Michael Stahl mstahl at redhat.com
Tue Jan 19 07:41:43 PST 2016


 include/xmloff/txtimp.hxx                       |    3 +
 sw/inc/IDocumentMarkAccess.hxx                  |    2 
 sw/qa/extras/odfexport/odfexport.cxx            |    4 -
 sw/source/core/doc/docbm.cxx                    |    6 --
 sw/source/core/inc/MarkManager.hxx              |    2 
 sw/source/core/unocore/unobkm.cxx               |    9 ---
 xmloff/source/core/xmlimp.cxx                   |    2 
 xmloff/source/text/XMLTextMarkImportContext.cxx |   46 +++++++++++++++++-
 xmloff/source/text/XMLTextMarkImportContext.hxx |    6 ++
 xmloff/source/text/txtimp.cxx                   |   60 ++++++++++++++++++++++++
 xmloff/source/text/txtparai.cxx                 |   40 ++++++++++++----
 11 files changed, 152 insertions(+), 28 deletions(-)

New commits:
commit 55d2301ac658167396bf5533c940bceffff67f04
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Jan 8 16:02:43 2016 +0100

    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.
    
    (cherry picked from commit 774fb6d2e7cf36b677e66c54278225b1256bd40f)
    
    Change-Id: I7ee4854d1c9d8bf74201089cbb7287b1bd8ee3b9
    Reviewed-on: https://gerrit.libreoffice.org/21369
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>

diff --git a/include/xmloff/txtimp.hxx b/include/xmloff/txtimp.hxx
index ca0ce7a..291998b 100644
--- a/include/xmloff/txtimp.hxx
+++ b/include/xmloff/txtimp.hxx
@@ -733,6 +733,9 @@ public:
 
     void SetCellParaStyleDefault(OUString const& rNewValue);
     OUString const& GetCellParaStyleDefault();
+
+    void AddCrossRefHeadingMapping(OUString const& rFrom, OUString const& rTo);
+    void MapCrossRefHeadingFieldsHorribly();
 };
 
 #endif
diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index befbaeb..78e73f7 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 bc7d768..0e106fa 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -385,9 +385,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 4680744..9b93a37 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -352,8 +352,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
         {
@@ -376,8 +375,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 05e3cec..b797764 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -36,7 +36,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) SAL_OVERRIDE;
+            virtual ::sw::mark::IMark* makeMark(const SwPaM& rPaM, const OUString& rName, IDocumentMarkAccess::MarkType eMark) SAL_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 fd85f3f..e6bc9af 100644
--- a/sw/source/core/unocore/unobkm.cxx
+++ b/sw/source/core/unocore/unobkm.cxx
@@ -226,7 +226,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";
@@ -241,16 +240,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,
diff --git a/xmloff/source/core/xmlimp.cxx b/xmloff/source/core/xmlimp.cxx
index 46fdae8..6491db6 100644
--- a/xmloff/source/core/xmlimp.cxx
+++ b/xmloff/source/core/xmlimp.cxx
@@ -547,6 +547,8 @@ void SAL_CALL SvXMLImport::endDocument()
     //  #i9518# All the stuff that accesses the document has to be done here, not in the dtor,
     //  because the SvXMLImport dtor might not be called until after the document has been closed.
 
+    GetTextImport()->MapCrossRefHeadingFieldsHorribly();
+
     if (mpImpl->mpRDFaHelper.get())
     {
         const uno::Reference<rdf::XRepositorySupplier> xRS(mxModel,
diff --git a/xmloff/source/text/XMLTextMarkImportContext.cxx b/xmloff/source/text/XMLTextMarkImportContext.cxx
index f0d3458..f344cde 100644
--- a/xmloff/source/text/XMLTextMarkImportContext.cxx
+++ b/xmloff/source/text/XMLTextMarkImportContext.cxx
@@ -100,10 +100,12 @@ TYPEINIT1( XMLTextMarkImportContext, SvXMLImportContext);
 XMLTextMarkImportContext::XMLTextMarkImportContext(
     SvXMLImport& rImport,
     XMLTextImportHelper& rHlp,
+    uno::Reference<uno::XInterface> & io_rxCrossRefHeadingBookmark,
     sal_uInt16 nPrefix,
     const OUString& rLocalName )
     : SvXMLImportContext(rImport, nPrefix, rLocalName)
     , m_rHelper(rHlp)
+    , m_rxCrossRefHeadingBookmark(io_rxCrossRefHeadingBookmark)
     , m_bHaveAbout(false)
 {
 }
@@ -203,9 +205,23 @@ void XMLTextMarkImportContext::EndElement()
                         OUString());
                     break;
 
-                case TypeFieldmark:
                 case TypeBookmark:
                     {
+                        // tdf#94804: detect duplicate heading cross reference bookmarks
+                        if (m_sBookmarkName.startsWith("__RefHeading__"))
+                        {
+                            if (m_rxCrossRefHeadingBookmark.is())
+                            {
+                                uno::Reference<container::XNamed> const xNamed(
+                                    m_rxCrossRefHeadingBookmark, uno::UNO_QUERY);
+                                m_rHelper.AddCrossRefHeadingMapping(
+                                    m_sBookmarkName, xNamed->getName());
+                                break; // don't insert
+                            }
+                        }
+                    } // fall through
+                case TypeFieldmark:
+                    {
                         const char *formFieldmarkName=lcl_getFormFieldmarkName(m_sFieldName);
                         bool bImportAsField=((lcl_MarkType)nTmp==TypeFieldmark && formFieldmarkName!=NULL); //@TODO handle abbreviation cases..
                         // export point bookmark
@@ -226,6 +242,12 @@ void XMLTextMarkImportContext::EndElement()
                             }
                             m_rHelper.popFieldCtx();
                         }
+                        if (TypeBookmark == nTmp
+                            && m_sBookmarkName.startsWith("__RefHeading__"))
+                        {
+                            assert(xContent.is());
+                            m_rxCrossRefHeadingBookmark = xContent;
+                        }
                     }
                     break;
 
@@ -250,8 +272,22 @@ void XMLTextMarkImportContext::EndElement()
                     }
                     break;
 
-                case TypeFieldmarkEnd:
                 case TypeBookmarkEnd:
+                    {
+                        // tdf#94804: detect duplicate heading cross reference bookmarks
+                        if (m_sBookmarkName.startsWith("__RefHeading__"))
+                        {
+                            if (m_rxCrossRefHeadingBookmark.is())
+                            {
+                                uno::Reference<container::XNamed> const xNamed(
+                                    m_rxCrossRefHeadingBookmark, uno::UNO_QUERY);
+                                m_rHelper.AddCrossRefHeadingMapping(
+                                    m_sBookmarkName, xNamed->getName());
+                                break; // don't insert
+                            }
+                        }
+                    } // fall through
+                case TypeFieldmarkEnd:
                 {
                     // get old range, and construct
                     Reference<XTextRange> xStartRange;
@@ -334,6 +370,12 @@ void XMLTextMarkImportContext::EndElement()
                                 }
                                 m_rHelper.popFieldCtx();
                             }
+                            if (TypeBookmarkEnd == nTmp
+                                && m_sBookmarkName.startsWith("__RefHeading__"))
+                            {
+                                assert(xContent.is());
+                                m_rxCrossRefHeadingBookmark = xContent;
+                            }
                         }
                         // else: beginning/end in different XText -> ignore!
                     }
diff --git a/xmloff/source/text/XMLTextMarkImportContext.hxx b/xmloff/source/text/XMLTextMarkImportContext.hxx
index ae01b79..2f449ca 100644
--- a/xmloff/source/text/XMLTextMarkImportContext.hxx
+++ b/xmloff/source/text/XMLTextMarkImportContext.hxx
@@ -61,8 +61,11 @@ public:
  */
 class XMLTextMarkImportContext : public SvXMLImportContext
 {
-
+private:
     XMLTextImportHelper & m_rHelper;
+
+    css::uno::Reference<css::uno::XInterface> & m_rxCrossRefHeadingBookmark;
+
     OUString m_sBookmarkName;
     OUString m_sFieldName;
     OUString m_sXmlId;
@@ -80,6 +83,7 @@ public:
     XMLTextMarkImportContext(
         SvXMLImport& rImport,
         XMLTextImportHelper& rHlp,
+        css::uno::Reference<css::uno::XInterface> & io_rxCrossRefHeadingBookmark,
         sal_uInt16 nPrfx,
         const OUString& rLocalName );
 
diff --git a/xmloff/source/text/txtimp.cxx b/xmloff/source/text/txtimp.cxx
index 652f2b9..eb87ec3 100644
--- a/xmloff/source/text/txtimp.cxx
+++ b/xmloff/source/text/txtimp.cxx
@@ -25,7 +25,9 @@
 #include <com/sun/star/container/XEnumerationAccess.hpp>
 #include <com/sun/star/lang/XMultiServiceFactory.hpp>
 #include <com/sun/star/style/XStyleFamiliesSupplier.hpp>
+#include <com/sun/star/text/ReferenceFieldSource.hpp>
 #include <com/sun/star/text/XChapterNumberingSupplier.hpp>
+#include <com/sun/star/text/XTextFieldsSupplier.hpp>
 #include <com/sun/star/text/XTextFramesSupplier.hpp>
 #include <com/sun/star/text/XTextGraphicObjectsSupplier.hpp>
 #include <com/sun/star/text/XTextEmbeddedObjectsSupplier.hpp>
@@ -576,6 +578,8 @@ struct XMLTextImportHelper::Impl
 
     OUString m_sCellParaStyleDefault;
 
+    std::unique_ptr<std::map<OUString, OUString>> m_pCrossRefHeadingBookmarkMap;
+
     Impl(       uno::Reference<frame::XModel> const& rModel,
                 SvXMLImport & rImport,
                 bool const bInsertMode, bool const bStylesOnlyMode,
@@ -2801,4 +2805,60 @@ OUString const& XMLTextImportHelper::GetCellParaStyleDefault()
     return m_xImpl->m_sCellParaStyleDefault;
 }
 
+void XMLTextImportHelper::AddCrossRefHeadingMapping(OUString const& rFrom, OUString const& rTo)
+{
+    if (!m_xImpl->m_pCrossRefHeadingBookmarkMap)
+    {
+        m_xImpl->m_pCrossRefHeadingBookmarkMap.reset(new std::map<OUString, OUString>);
+    }
+    m_xImpl->m_pCrossRefHeadingBookmarkMap->insert(std::make_pair(rFrom, rTo));
+}
+
+// tdf#94804: hack to map cross reference fiels that reference duplicate marks
+// note that we can't really check meta:generator for this since the file might
+// be round-tripped by different versions preserving duplicates => always map
+void XMLTextImportHelper::MapCrossRefHeadingFieldsHorribly()
+{
+    if (!m_xImpl->m_pCrossRefHeadingBookmarkMap)
+    {
+        return;
+    }
+
+    uno::Reference<text::XTextFieldsSupplier> const xFieldsSupplier(
+            m_xImpl->m_rSvXMLImport.GetModel(), uno::UNO_QUERY);
+    if (!xFieldsSupplier.is())
+    {
+        return;
+    }
+    uno::Reference<container::XEnumerationAccess> const xFieldsEA(
+            xFieldsSupplier->getTextFields());
+    uno::Reference<container::XEnumeration> const xFields(
+            xFieldsEA->createEnumeration());
+    while (xFields->hasMoreElements())
+    {
+        uno::Reference<lang::XServiceInfo> const xFieldInfo(
+                xFields->nextElement(), uno::UNO_QUERY);
+        if (!xFieldInfo->supportsService("com.sun.star.text.textfield.GetReference"))
+        {
+            continue;
+        }
+        uno::Reference<beans::XPropertySet> const xField(
+                xFieldInfo, uno::UNO_QUERY);
+        sal_uInt16 nType(0);
+        xField->getPropertyValue("ReferenceFieldSource") >>= nType;
+        if (text::ReferenceFieldSource::BOOKMARK != nType)
+        {
+            continue;
+        }
+        OUString name;
+        xField->getPropertyValue("SourceName") >>= name;
+        auto const iter(m_xImpl->m_pCrossRefHeadingBookmarkMap->find(name));
+        if (iter == m_xImpl->m_pCrossRefHeadingBookmarkMap->end())
+        {
+            continue;
+        }
+        xField->setPropertyValue("SourceName", uno::makeAny(iter->second));
+    }
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/text/txtparai.cxx b/xmloff/source/text/txtparai.cxx
index 6a324b7..24853be 100644
--- a/xmloff/source/text/txtparai.cxx
+++ b/xmloff/source/text/txtparai.cxx
@@ -53,7 +53,6 @@
 #include "txtlists.hxx"
 
 #include <txtparaimphint.hxx>
-class XMLHints_Impl : public boost::ptr_vector<XMLHint_Impl> {};
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
@@ -66,6 +65,29 @@ using ::com::sun::star::container::XEnumeration;
 
 TYPEINIT1( XMLCharContext, SvXMLImportContext );
 
+class XMLHints_Impl
+{
+private:
+    boost::ptr_vector<XMLHint_Impl> m_Hints;
+    uno::Reference<uno::XInterface> m_xCrossRefHeadingBookmark;
+
+public:
+    void push_back(XMLHint_Impl * pHint)
+    {
+        m_Hints.push_back(pHint);
+    }
+
+    boost::ptr_vector<XMLHint_Impl> & GetHints()
+    {
+        return m_Hints;
+    }
+
+    uno::Reference<uno::XInterface> & GetCrossRefHeadingBookmark()
+    {
+        return m_xCrossRefHeadingBookmark;
+    }
+};
+
 XMLCharContext::XMLCharContext(
         SvXMLImport& rImport,
         sal_uInt16 nPrfx,
@@ -252,10 +274,10 @@ XMLEndReferenceContext_Impl::XMLEndReferenceContext_Impl(
     if (XMLStartReferenceContext_Impl::FindName(GetImport(), xAttrList, sName))
     {
         // search for reference start
-        sal_uInt16 nCount = rHints.size();
+        sal_uInt16 nCount = rHints.GetHints().size();
         for(sal_uInt16 nPos = 0; nPos < nCount; nPos++)
         {
-            XMLHint_Impl *pHint = &rHints[nPos];
+            XMLHint_Impl *pHint = &rHints.GetHints()[nPos];
             if ( pHint->IsReference() &&
                  sName.equals( static_cast<XMLReferenceHint_Impl *>(pHint)->GetRefName()) )
             {
@@ -1120,10 +1142,10 @@ void XMLIndexMarkImportContext_Impl::StartElement(
             if (!sID.isEmpty())
             {
                 // if we have an ID, find the hint and set the end position
-                sal_uInt16 nCount = rHints.size();
+                sal_uInt16 nCount = rHints.GetHints().size();
                 for(sal_uInt16 nPos = 0; nPos < nCount; nPos++)
                 {
-                    XMLHint_Impl *pHint = &rHints[nPos];
+                    XMLHint_Impl *pHint = &rHints.GetHints()[nPos];
                     if ( pHint->IsIndexMark() &&
                          sID.equals(
                              static_cast<XMLIndexMarkHint_Impl *>(pHint)->GetID()) )
@@ -1655,6 +1677,7 @@ SvXMLImportContext *XMLImpSpanContext_Impl::CreateChildContext(
     case XML_TOK_TEXT_BOOKMARK_END:
         pContext = new XMLTextMarkImportContext( rImport,
                                                  *rImport.GetTextImport().get(),
+                                                 rHints.GetCrossRefHeadingBookmark(),
                                                  nPrefix, rLocalName );
         break;
 
@@ -1663,6 +1686,7 @@ SvXMLImportContext *XMLImpSpanContext_Impl::CreateChildContext(
     case XML_TOK_TEXT_FIELDMARK_END:
         pContext = new XMLTextMarkImportContext( rImport,
                                                  *rImport.GetTextImport().get(),
+                                                 rHints.GetCrossRefHeadingBookmark(),
                                                  nPrefix, rLocalName );
         break;
 
@@ -2072,11 +2096,11 @@ XMLParaContext::~XMLParaContext()
         }
     }
 
-    if( pHints && !pHints->empty() )
+    if (pHints && !pHints->GetHints().empty())
     {
-        for( sal_uInt16 i=0; i<pHints->size(); i++ )
+        for( sal_uInt16 i=0; i < pHints->GetHints().size(); i++ )
         {
-            XMLHint_Impl *pHint = &(*pHints)[i];
+            XMLHint_Impl *pHint = &pHints->GetHints()[i];
             xAttrCursor->gotoRange( pHint->GetStart(), sal_False );
             xAttrCursor->gotoRange( pHint->GetEnd(), sal_True );
             switch( pHint->GetType() )


More information about the Libreoffice-commits mailing list