oox/source/export/drawingml.cxx | 101 +++++++++++++++++++++-------- sd/qa/unit/export-tests-ooxml3.cxx | 7 ++ sw/qa/extras/ooxmlexport/ooxmlexport13.cxx | 6 + 3 files changed, 88 insertions(+), 26 deletions(-)
New commits: commit cbc677310acf205cd4f6efd1fe1667b9c0f090a9 Author: [email protected] <[email protected]> AuthorDate: Sat Jan 10 18:14:30 2026 -0500 Commit: Justin Luth <[email protected]> CommitDate: Sat Jan 17 00:04:32 2026 +0100 tdf#166335 oox export: don't export invalid gluepoint fmla We started exporting gluepoints with 25.2.2 commit/86d36ee56521438069504fbacff8dc2aff3a1afc "tdf165262 PPTX export: fix shape export regression" and some of them were not in valid OOXML format. In this case, we were exporting an empty fmla="" which is seen as a corrupt document by MS Office. make CppunitTest_sd_export_tests-ooxml3 \ CPPUNIT_TEST_NAME=testTdf135843 make CppunitTest_sw_ooxmlexport13 CPPUNIT_TEST_NAME=testTdf123435 Change-Id: I4297d24b8e884199b8da0253ab855417b828aa75 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197003 Reviewed-by: Justin Luth <[email protected]> Tested-by: Jenkins (cherry picked from commit efeee389064ae7d56020fdbfe38a5e9cb48bc016) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197450 diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx index d5b0e761b356..7c74b6ea9c40 100644 --- a/oox/source/export/drawingml.cxx +++ b/oox/source/export/drawingml.cxx @@ -4974,18 +4974,44 @@ bool IsValidOOXMLFormula(std::u16string_view sFormula) return false; } -OUString GetFormula(const OUString& sEquation, const OUString& sReplace, const OUString& sNewStr) +OUString GetFormula(const OUString& sEquation) { + assert(!sEquation.isEmpty() && "surely an equation would never be empty..."); + // TODO: This needs to be completely re-written. It is extremely simplistic/minimal. + // What is needed here is the reverse of convertToOOEquation. + OUString sFormula = sEquation; - size_t nPos = sFormula.indexOf(sReplace); - if (nPos != std::string::npos) + + /* replace LO native placeholders with OOXML placeholders + * #1: e.g. 'logwidth' + * #2: e.g. 'logwidth/2' + * #3: e.g. '1234*logwidth/5678' + */ + + if (sEquation == "logwidth") // #1 + return "val w"; + if (sEquation == "logheight") + return "val h"; + if (sEquation.startsWith("logwidth/")) // #2 + sFormula = u"*/ 1 w "_ustr + sEquation.subView(9); + else if (sEquation.startsWith("logheight/")) + sFormula = u"*/ 1 h "_ustr + sEquation.subView(10); + else { - OUString sModifiedEquation = sFormula.replaceAt(nPos, sReplace.getLength(), sNewStr); - sFormula = "*/ " + sModifiedEquation; + size_t nPos = sFormula.indexOf("*logwidth/"); //#3 + if (nPos != std::string::npos) + sFormula = "*/ " + sFormula.replaceAt(nPos, 10, " w "); + else + { + nPos = sFormula.indexOf("*logheight/"); + if (nPos != std::string::npos) + sFormula = "*/ " + sFormula.replaceAt(nPos, 11, " h "); + } } if (IsValidOOXMLFormula(sFormula)) return sFormula; + else SAL_WARN("oox.shape","invalid OOXML formula["<<sFormula<<"]"); return OUString(); } @@ -4993,38 +5019,60 @@ OUString GetFormula(const OUString& sEquation, const OUString& sReplace, const O void prepareGluePoints(std::vector<Guide>& rGuideList, const css::uno::Sequence<OUString>& aEquations, const uno::Sequence<drawing::EnhancedCustomShapeParameterPair>& rGluePoints, - const bool bIsOOXML, const sal_Int32 nWidth, const sal_Int32 nHeight) + const bool /*bIsOOXML*/, const sal_Int32 nWidth, const sal_Int32 nHeight) { if (rGluePoints.hasElements()) { - sal_Int32 nIndex = 1; + sal_Int32 nIndex = 0; for (auto const& rGluePoint : rGluePoints) { + ++nIndex; sal_Int32 nIdx1 = -1; sal_Int32 nIdx2 = -1; - rGluePoint.First.Value >>= nIdx1; - rGluePoint.Second.Value >>= nIdx2; + bool bValidIdx1 = false; + bool bValidIdx2 = false; + if (rGluePoint.First.Value >>= nIdx1) + { + bValidIdx1 = rGluePoint.First.Type == EnhancedCustomShapeParameterType::EQUATION; + // I would assume that any EQUATION must define a valid index value. + assert(!bValidIdx1 || (nIdx1 >= 0 && nIdx1 < aEquations.getLength())); + } + else + continue; - if (nIdx1 != -1 && nIdx2 != -1) + if (rGluePoint.Second.Value >>= nIdx2) { - Guide aGuideX; - aGuideX.sName = "GluePoint"_ostr + OString::number(nIndex) + "X"; - aGuideX.sFormula - = (bIsOOXML && nIdx1 >= 0 && nIdx1 < aEquations.getLength()) - ? GetFormula(aEquations[nIdx1], "*logwidth/", " w ").toUtf8() - : "*/ " + OString::number(nIdx1) + " w " + OString::number(nWidth); - rGuideList.push_back(aGuideX); - - Guide aGuideY; - aGuideY.sName = "GluePoint"_ostr + OString::number(nIndex) + "Y"; - aGuideY.sFormula - = (bIsOOXML && nIdx2 >= 0 && nIdx2 < aEquations.getLength()) - ? GetFormula(aEquations[nIdx2], "*logheight/", " h ").toUtf8() - : "*/ " + OString::number(nIdx2) + " h " + OString::number(nHeight); - rGuideList.push_back(aGuideY); + bValidIdx2 = rGluePoint.Second.Type == EnhancedCustomShapeParameterType::EQUATION; + assert(!bValidIdx2 || (nIdx2 >= 0 && nIdx2 < aEquations.getLength())); } + else + continue; + + Guide aGuideX; + Guide aGuideY; + if (bValidIdx1) + { + aGuideX.sFormula = GetFormula(aEquations[nIdx1]).toUtf8(); + if (aGuideX.sFormula.isEmpty()) // !IsValidOOXMLFormula + continue; + } + else + aGuideX.sFormula = "*/ " + OString::number(nIdx1) + " w " + OString::number(nWidth); + if (bValidIdx2) + { + aGuideY.sFormula = GetFormula(aEquations[nIdx2]).toUtf8(); + if (aGuideY.sFormula.isEmpty()) // !IsValidOOXMLFormula + continue; + } + else + aGuideY.sFormula + = "*/ " + OString::number(nIdx2) + " h " + OString::number(nHeight); + + aGuideX.sName = "GluePoint"_ostr + OString::number(nIndex) + "X"; + rGuideList.push_back(aGuideX); - nIndex++; + aGuideY.sName = "GluePoint"_ostr + OString::number(nIndex) + "Y"; + rGuideList.push_back(aGuideY); } } } @@ -5218,6 +5266,7 @@ bool DrawingML::WriteCustomGeometry( mpFS->startElementNS(XML_a, XML_gdLst); for (auto const& elem : aGuideList) { + assert(IsValidOOXMLFormula(OUString::fromUtf8(elem.sFormula))); mpFS->singleElementNS(XML_a, XML_gd, XML_name, elem.sName, XML_fmla, elem.sFormula); } mpFS->endElementNS(XML_a, XML_gdLst); diff --git a/sd/qa/unit/export-tests-ooxml3.cxx b/sd/qa/unit/export-tests-ooxml3.cxx index ea66d313e482..2f67a226cf03 100644 --- a/sd/qa/unit/export-tests-ooxml3.cxx +++ b/sd/qa/unit/export-tests-ooxml3.cxx @@ -1071,6 +1071,13 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest3, testTdf135843) assertXPath(pXmlDoc, sPathStart + "/a:tr[3]/a:tc[1]/a:tcPr/a:lnR/a:solidFill"); assertXPath(pXmlDoc, sPathStart + "/a:tr[3]/a:tc[1]/a:tcPr/a:lnT/a:solidFill"); assertXPath(pXmlDoc, sPathStart + "/a:tr[3]/a:tc[1]/a:tcPr/a:lnB/a:solidFill"); + + // tdf#166335: fmla must not be an empty string + static constexpr OString sGlueStart("/p:sld/p:cSld/p:spTree/p:sp[3]/p:spPr/a:custGeom"_ostr); + assertXPath(pXmlDoc, sGlueStart + "/a:gdLst/a:gd[@name='GluePoint2X']", "fmla", u"val w"); + assertXPath(pXmlDoc, sGlueStart + "/a:gdLst/a:gd[@name='GluePoint2Y']", "fmla", u"*/ 1 h 2"); + assertXPath(pXmlDoc, sGlueStart + "/a:gdLst/a:gd[@name='GluePoint3X']", "fmla", u"*/ 1 w 2"); + assertXPath(pXmlDoc, sGlueStart + "/a:gdLst/a:gd[@name='GluePoint3Y']", "fmla", u"val h"); } CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest3, testNegativeTimeAnimateValue) diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx index 43c16d0c912d..75e6ae776e66 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx @@ -671,6 +671,12 @@ DECLARE_OOXMLEXPORT_TEST(testTdf123435, "tdf123435.docx") // - Expected: 2 // - Actual : 1 CPPUNIT_ASSERT_EQUAL(2, getShapes()); + + // tdf#166335: fmla must not be an empty string + save(TestFilter::DOCX); + xmlDocUniquePtr pXmlDoc = parseExport(u"word/document.xml"_ustr); + // without the fix, this was 8. Having an a:gd fmla="" causes MSOffice to report a corrupt doc. + assertXPath(pXmlDoc, "//a:gdLst/a:gd[@fmla='']", 0); } CPPUNIT_TEST_FIXTURE(Test, testTdf116371)
