include/oox/export/drawingml.hxx | 3 + oox/source/export/drawingml.cxx | 7 +++- sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx |binary sd/qa/unit/export-tests-ooxml4.cxx | 31 +++++++++++++++++++ sd/source/filter/eppt/epptooxml.hxx | 6 ++- sd/source/filter/eppt/pptx-epptooxml.cxx | 10 ++++-- 6 files changed, 49 insertions(+), 8 deletions(-)
New commits: commit ac258f17f86ef23841980f08e1c3d97e9c46553c Author: Aron Budea <[email protected]> AuthorDate: Thu Nov 6 00:59:25 2025 +1030 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Nov 10 13:47:43 2025 +0100 tdf#168755 sd smartart: use unified ids for diagrams and shapes Diagrams (SmartArt) had their separate counters for ids, and when saving animation details, the shape to target wasn't found, resulting in tags like this, where negative spid is invalid: <p:spTgt spid="-1"/> See "19.5.72 spTgt (Shape Target)" in OOXML spec. Ids can't be completely unified, as it would affect naming in ppt/diagrams and on the UI to have large numbers (eg. in the 40-60 range), while users would expect them to start from 1. At least ids to "p:cNvPr" under "p:graphicFrame" should be unified. The large number is because shapes are numbered consecutively over all slides, while for the most part they could restart on each slide, changing that is not in scope for this fix. Change-Id: I9790036cc0743313e98f5926d4dc365c8051bd04 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193479 Reviewed-by: Mike Kaganski <[email protected]> Reviewed-by: Aron Budea <[email protected]> Tested-by: Jenkins (cherry picked from commit 2de71234b3829672505cf8f6222c27eaf4f0b7af) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193564 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx index 810fc4ff492e..9d2c9d76b2d4 100644 --- a/include/oox/export/drawingml.hxx +++ b/include/oox/export/drawingml.hxx @@ -494,7 +494,8 @@ public: OOX_DLLPUBLIC void Write3DEffects(const css::uno::Reference<css::beans::XPropertySet>& rXPropSet, bool bIsText); void WriteArtisticEffect( const css::uno::Reference< css::beans::XPropertySet >& rXPropSet ); OString WriteWdpPicture( const OUString& rFileId, const css::uno::Sequence< sal_Int8 >& rPictureData ); - OOX_DLLPUBLIC void WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId); + OOX_DLLPUBLIC void WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId, sal_Int32 nShapeId = -1); void writeDiagramRels(const css::uno::Sequence<css::uno::Sequence<css::uno::Any>>& xRelSeq, const css::uno::Reference<css::io::XOutputStream>& xOutStream, std::u16string_view sGrabBagProperyName, int nDiagramId); diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx index 9098e6d9d4bd..12c0ab619abf 100644 --- a/oox/source/export/drawingml.cxx +++ b/oox/source/export/drawingml.cxx @@ -6496,7 +6496,7 @@ OString DrawingML::WriteWdpPicture( const OUString& rFileId, const Sequence< sal return OUStringToOString(aId, RTL_TEXTENCODING_UTF8); } -void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId) +void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, sal_Int32 nDiagramId, sal_Int32 nShapeId) { uno::Reference<beans::XPropertySet> xPropSet(rXShape, uno::UNO_QUERY); @@ -6540,7 +6540,10 @@ void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rX // generate a unique id rtl::Reference<sax_fastparser::FastAttributeList> pDocPrAttrList = sax_fastparser::FastSerializerHelper::createAttrList(); - pDocPrAttrList->add(XML_id, OString::number(nDiagramId)); + // id in cNvPr under cNvGraphicFramePr must be unique among shapes, + // but can be different from diagram's own id + const sal_Int32 nExpShapeId = nShapeId != -1 ? nShapeId : nDiagramId; + pDocPrAttrList->add(XML_id, OString::number(nExpShapeId)); OString sName = "Diagram" + OString::number(nDiagramId); pDocPrAttrList->add(XML_name, sName); diff --git a/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx b/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx new file mode 100644 index 000000000000..8003a1f50318 Binary files /dev/null and b/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx differ diff --git a/sd/qa/unit/export-tests-ooxml4.cxx b/sd/qa/unit/export-tests-ooxml4.cxx index d9bea30c1334..db6933a04cf6 100644 --- a/sd/qa/unit/export-tests-ooxml4.cxx +++ b/sd/qa/unit/export-tests-ooxml4.cxx @@ -114,6 +114,37 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testSmartArtPreserve) "ContentType", u"application/vnd.openxmlformats-officedocument.drawingml.diagramStyle+xml"); } +CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testTdf168755_anim_on_SmartArt) +{ + createSdImpressDoc("pptx/tdf168755_anim_on_SmartArt.pptx"); + save(u"Impress Office Open XML"_ustr); + + xmlDocUniquePtr pXmlDocContent = parseExport(u"ppt/slides/slide1.xml"_ustr); + + const OUString aDiagramId = getXPath( + pXmlDocContent, "//p:sld/p:cSld/p:spTree/p:graphicFrame/p:nvGraphicFramePr/p:cNvPr", "id"); + OUString aSpTgtId + = getXPath(pXmlDocContent, + "//p:sld/p:timing/p:tnLst/p:par/p:cTn/p:childTnLst/p:seq/p:cTn/p:childTnLst/" + "p:par/p:cTn/p:childTnLst/p:par/p:cTn/p:childTnLst/p:par/p:cTn/" + "p:childTnLst/p:set/p:cBhvr/p:tgtEl/p:spTgt", + "spid"); + // Before the fix this would fail, as the "spid" attribute of the target was -1 + CPPUNIT_ASSERT_MESSAGE("Shape id in animation target can't be -1", aSpTgtId != "-1"); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Shape id in animation target differs from original", aDiagramId, + aSpTgtId); + // Repeat the same with a slightly different path (contains "animEffect" instead of "set") + aSpTgtId + = getXPath(pXmlDocContent, + "//p:sld/p:timing/p:tnLst/p:par/p:cTn/p:childTnLst/p:seq/p:cTn/p:childTnLst/" + "p:par/p:cTn/p:childTnLst/p:par/p:cTn/p:childTnLst/p:par/p:cTn/" + "p:childTnLst/p:animEffect/p:cBhvr/p:tgtEl/p:spTgt", + "spid"); + CPPUNIT_ASSERT_MESSAGE("Shape id in animation target can't be -1", aSpTgtId != "-1"); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Shape id in animation target differs from original", aDiagramId, + aSpTgtId); +} + CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testTdf125346) { // There are two themes in the test document, make sure we use the right theme diff --git a/sd/source/filter/eppt/epptooxml.hxx b/sd/source/filter/eppt/epptooxml.hxx index 3db4dc9e15b8..2dea61ff2e6f 100644 --- a/sd/source/filter/eppt/epptooxml.hxx +++ b/sd/source/filter/eppt/epptooxml.hxx @@ -123,7 +123,9 @@ private: virtual OUString SAL_CALL getImplementationName() override; - static void WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId); + static void WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, + const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId); /** Create a new placeholder index for a master placeholder shape @@ -159,7 +161,7 @@ private: sal_uInt32 mnAnimationNodeIdMax; sal_uInt32 mnThemeIdMax; - sal_uInt32 mnDiagramId; + sal_Int32 mnDiagramId; std::vector<OUString> maRelId; diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx b/sd/source/filter/eppt/pptx-epptooxml.cxx index 8e939ec201fd..133909760d21 100644 --- a/sd/source/filter/eppt/pptx-epptooxml.cxx +++ b/sd/source/filter/eppt/pptx-epptooxml.cxx @@ -2850,11 +2850,15 @@ OUString PowerPointExport::getImplementationName() return u"com.sun.star.comp.Impress.oox.PowerPointExport"_ustr; } -void PowerPointExport::WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId) +void PowerPointExport::WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, + const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId) { - SAL_INFO("sd.eppt", "writing Diagram " + OUString::number(nDiagramId)); + sal_Int32 nShapeId = rDML.GetNewShapeID(rXShape); + SAL_INFO("sd.eppt", "writing Diagram " + OUString::number(nDiagramId) + " with Shape Id " + + OUString::number(nShapeId)); pFS->startElementNS(XML_p, XML_graphicFrame); - rDML.WriteDiagram(rXShape, nDiagramId); + rDML.WriteDiagram(rXShape, nDiagramId, nShapeId); pFS->endElementNS(XML_p, XML_graphicFrame); }
