oox/qa/unit/data/preset-adjust-value.pptx |binary oox/qa/unit/drawingml.cxx | 30 +++++++++++++++++++++++- oox/source/drawingml/customshapeproperties.cxx | 6 ++++ oox/source/drawingml/shapepropertiescontext.cxx | 5 ++++ 4 files changed, 39 insertions(+), 2 deletions(-)
New commits: commit f10861b24ab735a7659066071ff82763a054ce5c Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Jun 10 11:07:43 2020 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Mon Jun 15 09:53:05 2020 +0200 PPTX import: handle adjust values from both the shape and its placeholder The direct problem is a crash in CustomShapeProperties::pushToPropSet(), the code just hoped that the input file doesn't have more adjust values than the # of adjust values we allocate based on the preset type. Fix the crash, but there is a deeper problem here... The shape can inherit custom shape properties from a placeholder, then later it can have its own custom shape properties. When it comes to adjust values specifically, we used to just append own adjust values to the end of the list. This way we got the double of expected adjust values. And later rendering took the N expected adjust values from the start of the 2N element list, so we used the adjust values of the placeholder, not of the actual shape. Fix this by clearing the custom shape geometry once we know we have our own preset geometry. (cherry picked from commit 408ec7a4470741edbedbb034de07a2d776348593) Change-Id: I09f669bf59c33b552b906733d323eba7af5548e7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96059 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/oox/qa/unit/data/preset-adjust-value.pptx b/oox/qa/unit/data/preset-adjust-value.pptx new file mode 100644 index 000000000000..d1d570a19d0a Binary files /dev/null and b/oox/qa/unit/data/preset-adjust-value.pptx differ diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx index bc2f910bec9b..e1400c7bfe0c 100644 --- a/oox/qa/unit/drawingml.cxx +++ b/oox/qa/unit/drawingml.cxx @@ -16,6 +16,7 @@ #include <com/sun/star/drawing/XShape.hpp> #include <com/sun/star/frame/Desktop.hpp> #include <com/sun/star/frame/XStorable.hpp> +#include <com/sun/star/drawing/EnhancedCustomShapeAdjustmentValue.hpp> #include <unotools/mediadescriptor.hxx> #include <unotools/tempfile.hxx> @@ -50,6 +51,7 @@ public: void setUp() override; void tearDown() override; uno::Reference<lang::XComponent>& getComponent() { return mxComponent; } + void load(const OUString& rURL); void loadAndReload(const OUString& rURL, const OUString& rFilterName); }; @@ -68,9 +70,11 @@ void OoxDrawingmlTest::tearDown() test::BootstrapFixture::tearDown(); } +void OoxDrawingmlTest::load(const OUString& rURL) { mxComponent = loadFromDesktop(rURL); } + void OoxDrawingmlTest::loadAndReload(const OUString& rURL, const OUString& rFilterName) { - mxComponent = loadFromDesktop(rURL); + load(rURL); uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY); utl::MediaDescriptor aMediaDescriptor; aMediaDescriptor["FilterName"] <<= rFilterName; @@ -128,6 +132,30 @@ CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testTdf131082) CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_SOLID, eFillStyle); } +CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testPresetAdjustValue) +{ + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "preset-adjust-value.pptx"; + + load(aURL); + + uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY); + uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0), + uno::UNO_QUERY); + uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY); + uno::Sequence<beans::PropertyValue> aGeoPropSeq; + xShapeProps->getPropertyValue("CustomShapeGeometry") >>= aGeoPropSeq; + comphelper::SequenceAsHashMap aGeoPropMap(aGeoPropSeq); + uno::Sequence<drawing::EnhancedCustomShapeAdjustmentValue> aAdjustmentSeq; + aGeoPropMap.getValue("AdjustmentValues") >>= aAdjustmentSeq; + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), aAdjustmentSeq.getLength()); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 11587 + // - Actual : 10813 + // i.e. the adjust value was set from the placeholder, not from the shape. + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(11587), aAdjustmentSeq[0].Value.get<sal_Int32>()); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/oox/source/drawingml/customshapeproperties.cxx b/oox/source/drawingml/customshapeproperties.cxx index 3b530bddfed6..00ecf33368ae 100644 --- a/oox/source/drawingml/customshapeproperties.cxx +++ b/oox/source/drawingml/customshapeproperties.cxx @@ -203,7 +203,11 @@ void CustomShapeProperties::pushToPropSet( aAdjustmentVal.Value <<= adjustmentGuide.maFormula.toInt32(); aAdjustmentVal.State = PropertyState_DIRECT_VALUE; aAdjustmentVal.Name = adjustmentGuide.maName; - aAdjustmentSeq[ nIndex++ ] = aAdjustmentVal; + if (nIndex < aAdjustmentSeq.getLength()) + { + aAdjustmentSeq[nIndex] = aAdjustmentVal; + ++nIndex; + } } } rGeoProp.Value <<= aAdjustmentSeq; diff --git a/oox/source/drawingml/shapepropertiescontext.cxx b/oox/source/drawingml/shapepropertiescontext.cxx index bbe621786742..4591cb834489 100644 --- a/oox/source/drawingml/shapepropertiescontext.cxx +++ b/oox/source/drawingml/shapepropertiescontext.cxx @@ -28,6 +28,7 @@ #include <oox/helper/attributelist.hxx> #include <oox/token/namespaces.hxx> #include <oox/token/tokens.hxx> +#include <drawingml/customshapeproperties.hxx> using namespace oox::core; using namespace ::com::sun::star; @@ -71,6 +72,10 @@ ContextHandlerRef ShapePropertiesContext::onCreateContext( sal_Int32 aElementTok { mrShape.getServiceName() = "com.sun.star.drawing.CustomShape"; } + + // We got a preset geometry, forget the geometry inherited from the placeholder shape. + mrShape.getCustomShapeProperties() = std::make_shared<CustomShapeProperties>(); + return new PresetShapeGeometryContext( *this, rAttribs, *(mrShape.getCustomShapeProperties()) ); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits