[Libreoffice-commits] core.git: include/xmloff sd/qa xmloff/source
Caolán McNamara (via logerrit)
logerrit at kemper.freedesktop.org
Fri Aug 21 09:36:48 UTC 2020
include/xmloff/unointerfacetouniqueidentifiermapper.hxx | 21 +-
sd/qa/unit/data/odg/rhbz1870501.odg |binary
sd/qa/unit/export-tests.cxx | 9 +
xmloff/source/core/unointerfacetouniqueidentifiermapper.cxx | 97 +++++++-----
xmloff/source/draw/ximpshap.cxx | 9 -
5 files changed, 88 insertions(+), 48 deletions(-)
New commits:
commit 3c230d608b766e573eeca66e203114b262478230
Author: Caolán McNamara <caolanm at redhat.com>
AuthorDate: Thu Aug 20 12:56:36 2020 +0100
Commit: Caolán McNamara <caolanm at redhat.com>
CommitDate: Fri Aug 21 11:36:05 2020 +0200
rhbz#1870501 crash on reexport of odg
where SdrObjects in a list have no navigation position set
a regression from
450cd772aa734cfcb989c8cedd3c0a454db74a34
Fix fdo#64512 Handle xml:id correctly on multi-image draw:frames
this just reverts that and adds the test case that crashed
Change-Id: I1a49dab9578699c42fe845e8ec42de40159dec3d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101074
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
diff --git a/include/xmloff/unointerfacetouniqueidentifiermapper.hxx b/include/xmloff/unointerfacetouniqueidentifiermapper.hxx
index a78713f85d13..332005e720ac 100644
--- a/include/xmloff/unointerfacetouniqueidentifiermapper.hxx
+++ b/include/xmloff/unointerfacetouniqueidentifiermapper.hxx
@@ -24,6 +24,7 @@
#include <xmloff/dllapi.h>
#include <sal/types.h>
+#include <deque>
#include <map>
#include <rtl/ustring.hxx>
#include <com/sun/star/uno/XInterface.hpp>
@@ -35,6 +36,8 @@ typedef ::std::map< OUString, css::uno::Reference< css::uno::XInterface > > IdMa
class XMLOFF_DLLPUBLIC UnoInterfaceToUniqueIdentifierMapper
{
+ typedef std::deque< OUString > Reserved_t;
+
public:
UnoInterfaceToUniqueIdentifierMapper();
@@ -50,12 +53,16 @@ public:
*/
bool registerReference( const OUString& rIdentifier, const css::uno::Reference< css::uno::XInterface >& rInterface );
- /** always registers the given uno object with the given identifier.
+ /** reserves an identifier for later registration.
- In contrast to registerReference(), this here overwrites any
- earlier registration of the same identifier
- */
- void registerReferenceAlways( const OUString& rIdentifier, const css::uno::Reference< css::uno::XInterface >& rInterface );
+ @returns
+ false, if the identifier already exists
+ */
+ bool reserveIdentifier( const OUString& rIdentifier );
+
+ /** registers the given uno object with reserved identifier.
+ */
+ bool registerReservedReference( const OUString& rIdentifier, const css::uno::Reference< css::uno::XInterface >& rInterface );
/** @returns
the identifier for the given uno object. If this uno object is not already
@@ -72,10 +79,12 @@ public:
private:
bool findReference( const css::uno::Reference< css::uno::XInterface >& rInterface, IdMap_t::const_iterator& rIter ) const;
bool findIdentifier( const OUString& rIdentifier, IdMap_t::const_iterator& rIter ) const;
- void insertReference( const OUString& rIdentifier, const css::uno::Reference< css::uno::XInterface >& rInterface );
+ bool findReserved( const OUString& rIdentifier ) const;
+ bool findReserved( const OUString& rIdentifier, Reserved_t::const_iterator& rIter ) const;
IdMap_t maEntries;
sal_uInt32 mnNextId;
+ Reserved_t maReserved;
};
}
diff --git a/sd/qa/unit/data/odg/rhbz1870501.odg b/sd/qa/unit/data/odg/rhbz1870501.odg
new file mode 100644
index 000000000000..cc4bcef193c2
Binary files /dev/null and b/sd/qa/unit/data/odg/rhbz1870501.odg differ
diff --git a/sd/qa/unit/export-tests.cxx b/sd/qa/unit/export-tests.cxx
index eb97da60fd1f..1acda7b2712f 100644
--- a/sd/qa/unit/export-tests.cxx
+++ b/sd/qa/unit/export-tests.cxx
@@ -81,6 +81,7 @@ public:
void testSoftEdges();
void testShadowBlur();
void testTdf115753();
+ void testRhbz1870501();
CPPUNIT_TEST_SUITE(SdExportTest);
@@ -119,6 +120,7 @@ public:
CPPUNIT_TEST(testSoftEdges);
CPPUNIT_TEST(testShadowBlur);
CPPUNIT_TEST(testTdf115753);
+ CPPUNIT_TEST(testRhbz1870501);
CPPUNIT_TEST_SUITE_END();
@@ -1377,6 +1379,13 @@ void SdExportTest::testShadowBlur()
xDocShRef->DoClose();
}
+void SdExportTest::testRhbz1870501()
+{
+ //Without the fix in place, it would crash at export time
+ ::sd::DrawDocShellRef xDocShRef = loadURL( m_directories.getURLFromSrc("/sd/qa/unit/data/odg/rhbz1870501.odg"), ODG);
+ xDocShRef = saveAndReload( xDocShRef.get(), ODG );
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(SdExportTest);
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/xmloff/source/core/unointerfacetouniqueidentifiermapper.cxx b/xmloff/source/core/unointerfacetouniqueidentifiermapper.cxx
index d2e29422ae1c..ce54fe971d44 100644
--- a/xmloff/source/core/unointerfacetouniqueidentifiermapper.cxx
+++ b/xmloff/source/core/unointerfacetouniqueidentifiermapper.cxx
@@ -21,6 +21,7 @@
#include <o3tl/safeint.hxx>
#include <xmloff/unointerfacetouniqueidentifiermapper.hxx>
+#include <algorithm>
using namespace ::com::sun::star;
using css::uno::Reference;
@@ -64,25 +65,41 @@ bool UnoInterfaceToUniqueIdentifierMapper::registerReference( const OUString& rI
{
return rIdentifier != (*aIter).first;
}
- else if( findIdentifier( rIdentifier, aIter ) )
+ else if( findIdentifier( rIdentifier, aIter ) || findReserved( rIdentifier ) )
{
return false;
}
else
{
- insertReference( rIdentifier, xRef );
+ maEntries.insert( IdMap_t::value_type( rIdentifier, xRef ) );
+
+ // see if this is a reference like something we would generate in the future
+ const sal_Unicode *p = rIdentifier.getStr();
+ sal_Int32 nLength = rIdentifier.getLength();
+
+ // see if the identifier is 'id' followed by a pure integer value
+ if( nLength < 2 || p[0] != 'i' || p[1] != 'd' )
+ return true;
+
+ nLength -= 2;
+ p += 2;
+
+ while(nLength--)
+ {
+ if( (*p < '0') || (*p > '9') )
+ return true; // a custom id, that will never conflict with genereated id's
+ p++;
+ }
+
+ // the identifier is a pure integer value
+ // so we make sure we will never generate
+ // an integer value like this one
+ sal_Int32 nId = rIdentifier.copy(2).toInt32();
+ if (nId > 0 && mnNextId <= o3tl::make_unsigned(nId))
+ mnNextId = nId + 1;
+
+ return true;
}
-
- return true;
-}
-
-void UnoInterfaceToUniqueIdentifierMapper::registerReferenceAlways( const OUString& rIdentifier, const Reference< XInterface >& rInterface )
-{
- // Be certain that the references we store in our table are to the
- // leading / primary XInterface - cf. findReference
- uno::Reference< uno::XInterface > xRef( rInterface, uno::UNO_QUERY );
-
- insertReference( rIdentifier, xRef );
}
const OUString& UnoInterfaceToUniqueIdentifierMapper::getIdentifier( const Reference< XInterface >& rInterface ) const
@@ -135,38 +152,42 @@ bool UnoInterfaceToUniqueIdentifierMapper::findIdentifier( const OUString& rIden
return rIter != maEntries.end();
}
-void UnoInterfaceToUniqueIdentifierMapper::insertReference( const OUString& rIdentifier, const Reference< XInterface >& rInterface )
+bool UnoInterfaceToUniqueIdentifierMapper::reserveIdentifier( const OUString& rIdentifier )
{
- maEntries[rIdentifier] = rInterface;
+ if ( findReserved( rIdentifier ) )
+ return false;
- // see if this is a reference like something we would generate in the future
- const sal_Unicode *p = rIdentifier.getStr();
- sal_Int32 nLength = rIdentifier.getLength();
+ maReserved.push_back( rIdentifier );
+ return true;
+}
- // see if the identifier is 'id' followed by a pure integer value
- if( nLength < 2 || p[0] != 'i' || p[1] != 'd' )
- return;
+bool UnoInterfaceToUniqueIdentifierMapper::registerReservedReference(
+ const OUString& rIdentifier,
+ const css::uno::Reference< css::uno::XInterface >& rInterface )
+{
+ Reserved_t::const_iterator aIt;
+ if ( !findReserved( rIdentifier, aIt ) )
+ return false;
- nLength -= 2;
- p += 2;
+ Reserved_t::iterator aRemoveIt( maReserved.begin() + ( aIt - maReserved.begin() ) );
+ maReserved.erase( aRemoveIt );
+ registerReference( rIdentifier, rInterface );
- while(nLength--)
- {
- if( (*p < '0') || (*p > '9') )
- return; // a custom id, that will never conflict with generated id's
+ return true;
+}
- p++;
- }
+bool UnoInterfaceToUniqueIdentifierMapper::findReserved( const OUString& rIdentifier ) const
+{
+ Reserved_t::const_iterator aDummy;
+ return findReserved( rIdentifier, aDummy );
+}
- // the identifier is a pure integer value
- // so we make sure we will never generate
- // an integer value like this one
- sal_Int32 nId = rIdentifier.copy(2).toInt32();
- if (nId > 0 && mnNextId <= o3tl::make_unsigned(nId))
- {
- mnNextId = nId;
- ++mnNextId;
- }
+bool UnoInterfaceToUniqueIdentifierMapper::findReserved(
+ const OUString& rIdentifier,
+ Reserved_t::const_iterator& rIter ) const
+{
+ rIter = std::find( maReserved.begin(), maReserved.end(), rIdentifier );
+ return rIter != maReserved.end();
}
}
diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx
index c7cf5b4c08e2..e0e467b31501 100644
--- a/xmloff/source/draw/ximpshap.cxx
+++ b/xmloff/source/draw/ximpshap.cxx
@@ -3459,6 +3459,9 @@ SvXMLImportContextRef SdXMLFrameShapeContext::CreateChildContext( sal_uInt16 nPr
if(getSupportsMultipleContents() && dynamic_cast< SdXMLGraphicObjectShapeContext* >(xContext.get()))
{
+ if ( !maShapeId.isEmpty() )
+ GetImport().getInterfaceToIdentifierMapper().reserveIdentifier( maShapeId );
+
addContent(*mxImplContext);
}
}
@@ -3568,13 +3571,11 @@ void SdXMLFrameShapeContext::EndElement()
SvXMLImportContextRef const pSelectedContext(solveMultipleImages());
const SdXMLGraphicObjectShapeContext* pShapeContext(
dynamic_cast<const SdXMLGraphicObjectShapeContext*>(pSelectedContext.get()));
- if ( pShapeContext && !maShapeId.isEmpty() )
+ if ( pShapeContext )
{
- // fdo#64512 and fdo#60075 - make sure *this* shape is
- // registered for given ID
assert( mxImplContext.is() );
const uno::Reference< uno::XInterface > xShape( pShapeContext->getShape() );
- GetImport().getInterfaceToIdentifierMapper().registerReferenceAlways( maShapeId, xShape );
+ GetImport().getInterfaceToIdentifierMapper().registerReservedReference( maShapeId, xShape );
}
if( !mxImplContext.is() )
More information about the Libreoffice-commits
mailing list