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

[Miklos Vajna (via logerrit)]

 oox/qa/unit/data/smartart-groupshape.pptx |binary
 oox/qa/unit/drawingml.cxx                 |   19 +++++++++++++++++++
 oox/source/drawingml/shape.cxx            |   17 ++++++++++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

New commits:
commit 0397077f41e5045b5f946a28cf8d6b72679a30cd
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Aug 6 16:56:30 2021 +0200
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Aug 10 12:21:17 2021 +0200

    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 at collabora.com>
    Tested-by: Jenkins
    (cherry picked from commit 030cdbc7f8782eb196f09661bc2f116d790de9be)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120145
    Reviewed-by: Xisco Fauli <xiscofauli at libreoffice.org>
    (cherry picked from commit 84033de1bfe3e6951a1f563d85e40fb6d29ff3be)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120149
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120252
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>

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
Binary files /dev/null and b/oox/qa/unit/data/smartart-groupshape.pptx differ
diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx
index c7e72d458194..e1c665e22e86 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 a9388e8a4a70..6e8ae072af56 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -295,7 +295,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 );
             }
 
@@ -964,7 +969,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())