include/xmloff/shapeexport.hxx | 4 +-- sw/qa/extras/odfexport/data/tdf168980.docx |binary sw/qa/extras/odfexport/odfexport2.cxx | 14 ++++++++++ xmloff/source/draw/shapeexport.cxx | 38 ++++++----------------------- 4 files changed, 24 insertions(+), 32 deletions(-)
New commits: commit 11cfb31796a4df1d506eb3a6f7acf2daef5602eb Author: Mike Kaganski <[email protected]> AuthorDate: Tue Oct 21 16:17:22 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Nov 4 16:41:45 2025 +0500 tdf#168980: don't use shape Z-order to identify shapes The problem was wrong identification of the shape type, because of unreliable use of fragile Z-order. XMLShapeExport::exportShape used xShape's Z-order to get the info collected for current xShape. Value of Z-order is calculated in SwXShape::getPropertyValue; it takes into account text boxes of shapes, which affects the correction in SwTextBoxHelper::getOrdNum. The correction results in several different objects returning the same Z-order. And in the specific document, the information that was collected for a rectangle object, was used for an object of different type. Instead of trying to find a smart way to continue using Z-order, this change uses XShape itself as the key into the map of info, similar to how XShapes objects are used as keys into ShapesInfos. Change-Id: Iacbc101623504db4290ad2c283351c4141b2d1b7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192796 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> (cherry picked from commit 0dc93fddcc751a28cfdb936f61bb6f0f4812e4b9) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192805 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/include/xmloff/shapeexport.hxx b/include/xmloff/shapeexport.hxx index 8398bb937837..035e1e3fb930 100644 --- a/include/xmloff/shapeexport.hxx +++ b/include/xmloff/shapeexport.hxx @@ -140,10 +140,10 @@ struct ImplXMLShapeExportInfo }; /** a vector for shape style and type cache information */ -typedef std::vector< ImplXMLShapeExportInfo > ImplXMLShapeExportInfoVector; +typedef std::map< css::uno::Reference< css::drawing::XShape >, ImplXMLShapeExportInfo > ShapeExportInfoMap; /** a map to store all cache data for already collected XShapes */ -typedef std::map< css::uno::Reference < css::drawing::XShapes >, ImplXMLShapeExportInfoVector > ShapesInfos; +typedef std::map< css::uno::Reference < css::drawing::XShapes >, ShapeExportInfoMap > ShapesInfos; class SvXMLExport; class SvXMLExportPropertyMapper; diff --git a/sw/qa/extras/odfexport/data/tdf168980.docx b/sw/qa/extras/odfexport/data/tdf168980.docx new file mode 100644 index 000000000000..c8f98f4638a2 Binary files /dev/null and b/sw/qa/extras/odfexport/data/tdf168980.docx differ diff --git a/sw/qa/extras/odfexport/odfexport2.cxx b/sw/qa/extras/odfexport/odfexport2.cxx index 8dff88773630..1881fd8ccbfe 100644 --- a/sw/qa/extras/odfexport/odfexport2.cxx +++ b/sw/qa/extras/odfexport/odfexport2.cxx @@ -843,6 +843,20 @@ CPPUNIT_TEST_FIXTURE(Test, testParagraphMarkerMarkupRoundtrip) assertXPath(pXmlDoc, "/office:document-content/office:automatic-styles/style:style[@style:name='T2']/style:text-properties", "color", "#ff0000"); } +CPPUNIT_TEST_FIXTURE(Test, testTdf168980) +{ + // In the document, an SvxCustomShape was exported to ODT as if it was a rectangle shape + createSwDoc("tdf168980.docx"); + // Before the fix, saving the document failed: + // Exception-Message: com.sun.star.beans.UnknownPropertyException: "CornerRadius at + // C:/lo/core/svx/source/unodraw/unoshape.cxx:1625" (context: class SvxCustomShape ) + // An uncaught UNO exception + // - com.sun.star.io.IOException: {Message: "SfxBaseModel::impl_store + // <file:///C:/lo/build/tmp/test_sw_odfexport4.dllizeba.tmp> failed: 0xc10(Error + // Area:Io Class:Write Code:16) + reload("OpenDocument Text Flat XML", ""); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/xmloff/source/draw/shapeexport.cxx b/xmloff/source/draw/shapeexport.cxx index 0f5b711a021a..a528fe701a23 100644 --- a/xmloff/source/draw/shapeexport.cxx +++ b/xmloff/source/draw/shapeexport.cxx @@ -154,7 +154,6 @@ bool supportsText(XmlShapeType eShapeType) } -constexpr OUStringLiteral gsZIndex( u"ZOrder" ); constexpr OUStringLiteral gsPrintable( u"Printable" ); constexpr OUStringLiteral gsVisible( u"Visible" ); constexpr OUStringLiteral gsModel( u"Model" ); @@ -269,20 +268,11 @@ void XMLShapeExport::collectShapeAutoStyles(const uno::Reference< drawing::XShap OSL_FAIL( "XMLShapeExport::collectShapeAutoStyles(): no call to seekShapes()!" ); return; } - sal_Int32 nZIndex = 0; uno::Reference< beans::XPropertySet > xPropSet(xShape, uno::UNO_QUERY); - if( xPropSet.is() ) - xPropSet->getPropertyValue(gsZIndex) >>= nZIndex; - - ImplXMLShapeExportInfoVector& aShapeInfoVector = (*maCurrentShapesIter).second; - if( static_cast<sal_Int32>(aShapeInfoVector.size()) <= nZIndex ) - { - OSL_FAIL( "XMLShapeExport::collectShapeAutoStyles(): no shape info allocated for a given shape" ); - return; - } + ShapeExportInfoMap& aShapeInfoMap = (*maCurrentShapesIter).second; - ImplXMLShapeExportInfo& aShapeInfo = aShapeInfoVector[nZIndex]; + ImplXMLShapeExportInfo& aShapeInfo = aShapeInfoMap[xShape]; uno::Reference< drawing::XShape > xCustomShapeReplacement = checkForCustomShapeReplacement( xShape ); if ( xCustomShapeReplacement.is() ) @@ -579,7 +569,6 @@ void XMLShapeExport::exportShape(const uno::Reference< drawing::XShape >& xShape SAL_WARN( "xmloff", "XMLShapeExport::exportShape(): no auto styles where collected before export" ); return; } - sal_Int32 nZIndex = 0; uno::Reference< beans::XPropertySet > xSet( xShape, uno::UNO_QUERY ); OUString sHyperlink; try @@ -635,12 +624,10 @@ void XMLShapeExport::exportShape(const uno::Reference< drawing::XShape >& xShape // re-add stashed attributes GetExport().AddAttributeList(xSaveAttribs); - if( xSet.is() ) - xSet->getPropertyValue(gsZIndex) >>= nZIndex; + ShapeExportInfoMap& aShapeInfoMap = (*maCurrentShapesIter).second; - ImplXMLShapeExportInfoVector& aShapeInfoVector = (*maCurrentShapesIter).second; - - if( static_cast<sal_Int32>(aShapeInfoVector.size()) <= nZIndex ) + auto it = aShapeInfoMap.find(xShape); + if (it == aShapeInfoMap.end()) { SAL_WARN( "xmloff", "XMLShapeExport::exportShape(): no shape info collected for a given shape" ); return; @@ -648,7 +635,7 @@ void XMLShapeExport::exportShape(const uno::Reference< drawing::XShape >& xShape NewTextListsHelper aNewTextListsHelper( mrExport ); - const ImplXMLShapeExportInfo& aShapeInfo = aShapeInfoVector[nZIndex]; + const ImplXMLShapeExportInfo& aShapeInfo = it->second; #ifdef DBG_UTIL // check if this is the correct ShapesInfo @@ -1089,17 +1076,8 @@ void XMLShapeExport::seekShapes( const uno::Reference< drawing::XShapes >& xShap { if( xShapes.is() ) { - maCurrentShapesIter = maShapesInfos.find( xShapes ); - if( maCurrentShapesIter == maShapesInfos.end() ) - { - ImplXMLShapeExportInfoVector aNewInfoVector; - aNewInfoVector.resize( static_cast<ShapesInfos::size_type>(xShapes->getCount()) ); - maShapesInfos[ xShapes ] = aNewInfoVector; - - maCurrentShapesIter = maShapesInfos.find( xShapes ); - - SAL_WARN_IF( maCurrentShapesIter == maShapesInfos.end(), "xmloff", "XMLShapeExport::seekShapes(): insert into stl::map failed" ); - } + maCurrentShapesIter = maShapesInfos.try_emplace(xShapes).first; + assert(maCurrentShapesIter != maShapesInfos.end()); SAL_WARN_IF( (*maCurrentShapesIter).second.size() != static_cast<ShapesInfos::size_type>(xShapes->getCount()), "xmloff", "XMLShapeExport::seekShapes(): XShapes size varied between calls" );
