mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
7c3c3006de 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
This commit is contained in:
Michael Stahl
2016-01-08 16:02:43 +01:00
parent 79bbc5b910
commit 774fb6d2e7
11 changed files with 150 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/xmloff/txtimp.hxx
View File

@@ -713,6 +713,9 @@ public:
void SetCellParaStyleDefault(OUString const& rNewValue);
OUString const& GetCellParaStyleDefault();
void AddCrossRefHeadingMapping(OUString const& rFrom, OUString const& rTo);
void MapCrossRefHeadingFieldsHorribly();
};
#endif

2
sw/inc/IDocumentMarkAccess.hxx
View File

@@ -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,

4
sw/qa/extras/odfexport/odfexport.cxx
View File

@@ -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();

6
sw/source/core/doc/docbm.cxx
View File

@@ -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");

2
sw/source/core/inc/MarkManager.hxx
View File

@@ -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,

9
sw/source/core/unocore/unobkm.cxx
View File

@@ -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,

2
xmloff/source/core/xmlimp.cxx
View File

@@ -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<rdf::XRepositorySupplier> xRS(mxModel,

46
xmloff/source/text/XMLTextMarkImportContext.cxx
View File

@@ -99,10 +99,12 @@ void XMLFieldParamImportContext::StartElement(const css::uno::Reference< css::xm
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)
{
}
@@ -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<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!=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<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;
@@ -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!
}

7
xmloff/source/text/XMLTextMarkImportContext.hxx
View File

@@ -60,8 +60,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;
@@ -74,10 +77,10 @@ class XMLTextMarkImportContext : public SvXMLImportContext
public:
XMLTextMarkImportContext(
SvXMLImport& rImport,
XMLTextImportHelper& rHlp,
css::uno::Reference<css::uno::XInterface> & io_rxCrossRefHeadingBookmark,
sal_uInt16 nPrfx,
const OUString& rLocalName );

60
xmloff/source/text/txtimp.cxx
View File

@@ -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>
@@ -575,6 +577,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,
@@ -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<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: */

38
xmloff/source/text/txtparai.cxx
View File

@@ -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<std::unique_ptr<XMLHint_Impl>> {};
class XMLHints_Impl
{
private:
std::vector<std::unique_ptr<XMLHint_Impl>> m_Hints;
uno::Reference<uno::XInterface> m_xCrossRefHeadingBookmark;
public:
void push_back(std::unique_ptr<XMLHint_Impl> pHint)
{
m_Hints.push_back(std::move(pHint));
}
std::vector<std::unique_ptr<XMLHint_Impl>> const& GetHints()
{
return m_Hints;
}
uno::Reference<uno::XInterface> & 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<XMLReferenceHint_Impl *>(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<XMLIndexMarkHint_Impl *>(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; i<pHints->size(); 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() )