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 844cc5b83216023a559ed87d365848d9909ef9de Author: Aron Budea <[email protected]> AuthorDate: Thu Nov 6 00:59:25 2025 +1030 Commit: Mike Kaganski <[email protected]> CommitDate: Fri Nov 7 10:48:53 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/+/193553 Tested-by: Jenkins CollaboraOffice <[email protected]> diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx index 1d104d7a3697..0d313311f43a 100644 --- a/include/oox/export/drawingml.hxx +++ b/include/oox/export/drawingml.hxx @@ -502,7 +502,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 cfde589b6562..fae6a06d4ca4 100644 --- a/oox/source/export/drawingml.cxx +++ b/oox/source/export/drawingml.cxx @@ -6581,7 +6581,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); @@ -6625,7 +6625,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 93eb86f859fc..dbc4e9674be6 100644 --- a/sd/qa/unit/export-tests-ooxml4.cxx +++ b/sd/qa/unit/export-tests-ooxml4.cxx @@ -115,6 +115,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 4c1ed63b0843..6d157b7ce68e 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 08fda508f7a5..11c9ea054bf5 100644 --- a/sd/source/filter/eppt/pptx-epptooxml.cxx +++ b/sd/source/filter/eppt/pptx-epptooxml.cxx @@ -2894,11 +2894,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); }
