[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.4' - oox/qa oox/source

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Wed Jun 10 19:12:16 UTC 2020


 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 3840fae38dd843a4ba543cbf26d10239624a8cb1
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Wed Jun 10 11:07:43 2020 +0200
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Wed Jun 10 21:11:41 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/+/96066
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Andras Timar <andras.timar at 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 6f9688be07f8..bf82a5cbc1cd 100644
--- a/oox/qa/unit/drawingml.cxx
+++ b/oox/qa/unit/drawingml.cxx
@@ -18,6 +18,7 @@
 #include <com/sun/star/text/XText.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
 #include <com/sun/star/frame/XStorable.hpp>
+#include <com/sun/star/drawing/EnhancedCustomShapeAdjustmentValue.hpp>
 
 #include <comphelper/embeddedobjectcontainer.hxx>
 #include <comphelper/processfactory.hxx>
@@ -58,6 +59,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);
 };
 
@@ -77,9 +79,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;
@@ -137,6 +141,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 2f4848088cc4..4a6e3d9eae6d 100644
--- a/oox/source/drawingml/customshapeproperties.cxx
+++ b/oox/source/drawingml/customshapeproperties.cxx
@@ -206,7 +206,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 ffedcfa5bf13..eedc338a25e4 100644
--- a/oox/source/drawingml/shapepropertiescontext.cxx
+++ b/oox/source/drawingml/shapepropertiescontext.cxx
@@ -34,6 +34,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;
@@ -77,6 +78,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()) );
         }
 


More information about the Libreoffice-commits mailing list