tdf#132696 PPTX import: fix missing SmartArt when it's part of a group shape

Regression from commit e9986153e44d7ec6ca9c5f1373971de74dcbacda (PPTX
import: import SmartArt drawing into single GroupShape, 2019-03-14), the
problem was that oox::ppt::PPTShape::addShape() and
oox::drawingml::Shape::addShape() were not in sync.

PPTShape unconditionally maps SmartArt to shapes, while the shared Shape
class defaults to converting it to a non-editable metafile. The above
commit changed the handling of in-groupshape SmartArts to go via
Shape::addShape() instead of PPTShape::addShape(), which exposed the
underlying problem that the convert-to-metafile mechanism is currently
only working in the DOCX case.

Fix the problem by again ignoring the convert-to-metafile flag for the
PPTX import case.

This also exposed a previously hidden problem:

make -C oox -sr CppunitTest_oox_drawingml CPPUNIT_TEST_NAME="testGroupShapeSmartArt testTdf131082"

started to make testTdf131082 fail. The tweak in
Shape::createAndInsert() fixes the testcase failure, but likely there is
a deeper problem there, unrelated to the regression.

Change-Id: I4e1e9645eaa266d0d7560767c3c59ba9549ccdb4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120122
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
(cherry picked from commit 030cdbc7f8782eb196f09661bc2f116d790de9be)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120145
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

3 files changed, 35 insertions, 1 deletions

diff --git a/oox/qa/unit/data/smartart-groupshape.pptx b/oox/qa/unit/data/smartart-groupshape.pptx
new file mode 100644
index 000000000000..81dcee1e52a3
--- /dev/null
+++ b/oox/qa/unit/data/smartart-groupshape.pptx
diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx
index 92d08b5f0c8a..c8dc0d9cc1fb 100644
--- a/oox/qa/unit/drawingml.cxx
+++ b/oox/qa/unit/drawingml.cxx
@@ -326,6 +326,25 @@ CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testTableShadow)
     verify(getComponent());
 }
 
+CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testGroupShapeSmartArt)
+{
+    // Given a file with a smartart inside a group shape:
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "smartart-groupshape.pptx";
+
+    // When loading that file:
+    load(aURL);
+
+    // Then make sure that the smartart is not just an empty group shape:
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+    uno::Reference<drawing::XShapes> xGroup(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+    uno::Reference<drawing::XShapes> xSmartArt(xGroup->getByIndex(0), uno::UNO_QUERY);
+    // Without the accompanying fix in place, this test would have failed, because we lost all
+    // children of the group shape representing the smartart.
+    CPPUNIT_ASSERT_GREATER(static_cast<sal_Int32>(0), xSmartArt->getCount());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index b0e2ddef30e7..b6900b8d1aeb 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -296,7 +296,12 @@ void Shape::addShape(
             if( meFrameType == FRAMETYPE_DIAGRAM )
             {
                 keepDiagramCompatibilityInfo();
-                if( !SvtFilterOptions::Get().IsSmartArt2Shape() )
+
+                // Check if this is the PPTX import, so far converting SmartArt to a non-editable
+                // metafile is only imlemented for DOCX.
+                bool bPowerPoint = dynamic_cast<oox::ppt::PowerPointImport*>(&rFilterBase) != nullptr;
+
+                if (!SvtFilterOptions::Get().IsSmartArt2Shape() && !bPowerPoint)
                     convertSmartArtToMetafile( rFilterBase );
             }
 
@@ -973,7 +978,17 @@ Reference< XShape > const & Shape::createAndInsert(
 
     Reference< lang::XMultiServiceFactory > xServiceFact( rFilterBase.getModel(), UNO_QUERY_THROW );
     if ( !mxShape.is() )
+    {
         mxShape.set( xServiceFact->createInstance( aServiceName ), UNO_QUERY_THROW );
+        if (aServiceName == "com.sun.star.drawing.GroupShape")
+        {
+            // TODO why is this necessary? A newly created group shape should have an empty
+            // grab-bag.
+            uno::Reference<beans::XPropertySet> xPropertySet(mxShape, uno::UNO_QUERY);
+            beans::PropertyValues aVals;
+            xPropertySet->setPropertyValue("InteropGrabBag", uno::makeAny(aVals));
+        }
+    }
 
     Reference< XPropertySet > xSet( mxShape, UNO_QUERY );
     if (xSet.is())