From 774fb6d2e7cf36b677e66c54278225b1256bd40f Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Fri, 8 Jan 2016 16:02:43 +0100 Subject: 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 --- 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 | 7 ++- xmloff/source/text/txtimp.cxx | 60 +++++++++++++++++++++++++ xmloff/source/text/txtparai.cxx | 38 ++++++++++++---- 11 files changed, 150 insertions(+), 29 deletions(-) diff --git a/include/xmloff/txtimp.hxx b/include/xmloff/txtimp.hxx index 6d9239f352ea..d91e3d0152dc 100644 --- a/include/xmloff/txtimp.hxx +++ b/include/xmloff/txtimp.hxx @@ -713,6 +713,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 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 xBookmark1( xBookmarks->getByName("__RefHeading__8284_1826734303"), uno::UNO_QUERY); CPPUNIT_ASSERT(xBookmark1.is()); - uno::Reference 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 xTextFieldsSupplier(mxComponent, uno::UNO_QUERY); uno::Reference(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, diff --git a/xmloff/source/core/xmlimp.cxx b/xmloff/source/core/xmlimp.cxx index 52b10a96d853..483cc02cb51f 100644 --- a/xmloff/source/core/xmlimp.cxx +++ b/xmloff/source/core/xmlimp.cxx @@ -545,6 +545,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 xRS(mxModel, diff --git a/xmloff/source/text/XMLTextMarkImportContext.cxx b/xmloff/source/text/XMLTextMarkImportContext.cxx index 2ebef98d77d6..a670fdc81390 100644 --- a/xmloff/source/text/XMLTextMarkImportContext.cxx +++ b/xmloff/source/text/XMLTextMarkImportContext.cxx @@ -99,10 +99,12 @@ void XMLFieldParamImportContext::StartElement(const css::uno::Reference< css::xm XMLTextMarkImportContext::XMLTextMarkImportContext( SvXMLImport& rImport, XMLTextImportHelper& rHlp, + uno::Reference & io_rxCrossRefHeadingBookmark, sal_uInt16 nPrefix, const OUString& rLocalName ) : SvXMLImportContext(rImport, nPrefix, rLocalName) , m_rHelper(rHlp) + , m_rxCrossRefHeadingBookmark(io_rxCrossRefHeadingBookmark) , m_bHaveAbout(false) { } @@ -202,8 +204,22 @@ 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 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!=nullptr); //@TODO handle abbreviation cases.. @@ -225,6 +241,12 @@ void XMLTextMarkImportContext::EndElement() } m_rHelper.popFieldCtx(); } + if (TypeBookmark == nTmp + && m_sBookmarkName.startsWith("__RefHeading__")) + { + assert(xContent.is()); + m_rxCrossRefHeadingBookmark = xContent; + } } break; @@ -249,8 +271,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 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 xStartRange; @@ -333,6 +369,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 b768c3567221..cc9bdf992330 100644 --- a/xmloff/source/text/XMLTextMarkImportContext.hxx +++ b/xmloff/source/text/XMLTextMarkImportContext.hxx @@ -60,8 +60,11 @@ public: */ class XMLTextMarkImportContext : public SvXMLImportContext { - +private: XMLTextImportHelper & m_rHelper; + + css::uno::Reference & m_rxCrossRefHeadingBookmark; + OUString m_sBookmarkName; OUString m_sFieldName; OUString m_sXmlId; @@ -74,10 +77,10 @@ class XMLTextMarkImportContext : public SvXMLImportContext public: - XMLTextMarkImportContext( SvXMLImport& rImport, XMLTextImportHelper& rHlp, + css::uno::Reference & io_rxCrossRefHeadingBookmark, sal_uInt16 nPrfx, const OUString& rLocalName ); diff --git a/xmloff/source/text/txtimp.cxx b/xmloff/source/text/txtimp.cxx index 2ece1f4b4d32..9afcb432d69f 100644 --- a/xmloff/source/text/txtimp.cxx +++ b/xmloff/source/text/txtimp.cxx @@ -25,7 +25,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -575,6 +577,8 @@ struct XMLTextImportHelper::Impl OUString m_sCellParaStyleDefault; + std::unique_ptr> m_pCrossRefHeadingBookmarkMap; + Impl( uno::Reference const& rModel, SvXMLImport & rImport, bool const bInsertMode, bool const bStylesOnlyMode, @@ -2797,4 +2801,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); + } + 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 const xFieldsSupplier( + m_xImpl->m_rSvXMLImport.GetModel(), uno::UNO_QUERY); + if (!xFieldsSupplier.is()) + { + return; + } + uno::Reference const xFieldsEA( + xFieldsSupplier->getTextFields()); + uno::Reference const xFields( + xFieldsEA->createEnumeration()); + while (xFields->hasMoreElements()) + { + uno::Reference const xFieldInfo( + xFields->nextElement(), uno::UNO_QUERY); + if (!xFieldInfo->supportsService("com.sun.star.text.textfield.GetReference")) + { + continue; + } + uno::Reference 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 ffceba4cfbbf..0d4b7e3c83e1 100644 --- a/xmloff/source/text/txtparai.cxx +++ b/xmloff/source/text/txtparai.cxx @@ -68,8 +68,28 @@ using namespace ::xmloff::token; using ::com::sun::star::container::XEnumerationAccess; using ::com::sun::star::container::XEnumeration; -class XMLHints_Impl : public std::vector> {}; +class XMLHints_Impl +{ +private: + std::vector> m_Hints; + uno::Reference m_xCrossRefHeadingBookmark; + +public: + void push_back(std::unique_ptr pHint) + { + m_Hints.push_back(std::move(pHint)); + } + std::vector> const& GetHints() + { + return m_Hints; + } + + uno::Reference & GetCrossRefHeadingBookmark() + { + return m_xCrossRefHeadingBookmark; + } +}; XMLCharContext::XMLCharContext( SvXMLImport& rImport, @@ -253,10 +273,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 *const pHint = rHints[nPos].get(); + XMLHint_Impl *const pHint = rHints.GetHints()[nPos].get(); if ( pHint->IsReference() && sName.equals( static_cast(pHint)->GetRefName()) ) { @@ -1102,10 +1122,10 @@ void XMLIndexMarkImportContext_Impl::StartElement( if (!sID.isEmpty()) { // if we have an ID, find the hint and set the end position - sal_uInt16 nCount = m_rHints.size(); + sal_uInt16 nCount = m_rHints.GetHints().size(); for(sal_uInt16 nPos = 0; nPos < nCount; nPos++) { - XMLHint_Impl *const pHint = m_rHints[nPos].get(); + XMLHint_Impl *const pHint = m_rHints.GetHints()[nPos].get(); if ( pHint->IsIndexMark() && sID.equals( static_cast(pHint)->GetID()) ) @@ -1629,6 +1649,7 @@ SvXMLImportContext *XMLImpSpanContext_Impl::CreateChildContext( case XML_TOK_TEXT_BOOKMARK_END: pContext = new XMLTextMarkImportContext( rImport, *rImport.GetTextImport().get(), + rHints.GetCrossRefHeadingBookmark(), nPrefix, rLocalName ); break; @@ -1637,6 +1658,7 @@ SvXMLImportContext *XMLImpSpanContext_Impl::CreateChildContext( case XML_TOK_TEXT_FIELDMARK_END: pContext = new XMLTextMarkImportContext( rImport, *rImport.GetTextImport().get(), + rHints.GetCrossRefHeadingBookmark(), nPrefix, rLocalName ); break; @@ -2045,11 +2067,11 @@ XMLParaContext::~XMLParaContext() } } - if( pHints && !pHints->empty() ) + if (pHints && !pHints->GetHints().empty()) { - for( size_t i=0; isize(); i++ ) + for (size_t i = 0; i < pHints->GetHints().size(); ++i) { - XMLHint_Impl *const pHint = (*pHints)[i].get(); + XMLHint_Impl *const pHint = pHints->GetHints()[i].get(); xAttrCursor->gotoRange( pHint->GetStart(), sal_False ); xAttrCursor->gotoRange( pHint->GetEnd(), sal_True ); switch( pHint->GetType() ) -- cgit v1.2.3