tdf#117658 PPTX import: fix duplicated math object handling

We used to recurse into both arms of <mc:AlternateContent>: while the
intention is that an importer either reads <mc:Choice> or <mc:Fallback>.

Fix this by converting PPTShapeGroupContext to be a FragmentHandler2,
this way FragmentHandler2::prepareMceContext() is invoked, which knows
how to do this correctly.

This requires declaring "a14" as a supported namespace, e.g.
SdOOXMLExportTest2::testMathObject() would fail without it.  This also
requires keeping "a14" unsupported in the Calc case, e.g.
ScFiltersTest::testControlImport() would fail without it.  Finally the
"Convert this to FragmentHandler2" TODO in
SlideFragmentHandler::onCreateContext() from 2011 can be removed with
this.

Change-Id: I883237902c71cb515e810a8e34443c9eeaca48b0
Reviewed-on: https://gerrit.libreoffice.org/82623
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

15 files changed, 55 insertions, 26 deletions

diff --git a/include/oox/drawingml/shapegroupcontext.hxx b/include/oox/drawingml/shapegroupcontext.hxx
index 1f9c860dc940..4fa313493d3e 100644
--- a/include/oox/drawingml/shapegroupcontext.hxx
+++ b/include/oox/drawingml/shapegroupcontext.hxx
@@ -21,7 +21,7 @@
 #define INCLUDED_OOX_DRAWINGML_SHAPEGROUPCONTEXT_HXX
 
 #include <oox/core/contexthandler.hxx>
-#include <oox/core/contexthandler2.hxx>
+#include <oox/core/fragmenthandler2.hxx>
 #include <oox/dllapi.h>
 #include <oox/drawingml/drawingmltypes.hxx>
 #include <sal/types.h>
@@ -30,10 +30,10 @@ namespace oox { class AttributeList; }
 
 namespace oox { namespace drawingml {
 
-class OOX_DLLPUBLIC ShapeGroupContext : public ::oox::core::ContextHandler2
+class OOX_DLLPUBLIC ShapeGroupContext : public ::oox::core::FragmentHandler2
 {
 public:
-    ShapeGroupContext( ::oox::core::ContextHandler2Helper const & rParent, ShapePtr const & pMasterShapePtr, ShapePtr const & pGroupShapePtr );
+    ShapeGroupContext( ::oox::core::FragmentHandler2 const & rParent, ShapePtr const & pMasterShapePtr, ShapePtr const & pGroupShapePtr );
     virtual ~ShapeGroupContext() override;
     virtual ::oox::core::ContextHandlerRef onCreateContext( ::sal_Int32 Element, const ::oox::AttributeList& rAttribs ) override;
 
diff --git a/include/oox/ppt/pptshapegroupcontext.hxx b/include/oox/ppt/pptshapegroupcontext.hxx
index 76b743ee16af..15667109d409 100644
--- a/include/oox/ppt/pptshapegroupcontext.hxx
+++ b/include/oox/ppt/pptshapegroupcontext.hxx
@@ -44,7 +44,7 @@ class PPTShapeGroupContext final : public ::oox::drawingml::ShapeGroupContext
 
 public:
     PPTShapeGroupContext(
-        ::oox::core::ContextHandler2Helper const & rParent,
+        ::oox::core::FragmentHandler2 const & rParent,
         const oox::ppt::SlidePersistPtr& rSlidePersistPtr,
         const oox::ppt::ShapeLocation eShapeLocation,
         const oox::drawingml::ShapePtr& pMasterShapePtr,
diff --git a/oox/qa/unit/data/import-mce.pptx b/oox/qa/unit/data/import-mce.pptx
new file mode 100644
index 000000000000..65cf1cbabfe7
--- /dev/null
+++ b/oox/qa/unit/data/import-mce.pptx
diff --git a/oox/qa/unit/mathml.cxx b/oox/qa/unit/mathml.cxx
index 632fc566ddb7..0352c7a84ec4 100644
--- a/oox/qa/unit/mathml.cxx
+++ b/oox/qa/unit/mathml.cxx
@@ -10,8 +10,9 @@
 #include <test/bootstrapfixture.hxx>
 #include <unotest/macros_test.hxx>
 
-#include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
 #include <com/sun/star/embed/XStorage.hpp>
+#include <com/sun/star/frame/Desktop.hpp>
 
 #include <comphelper/embeddedobjectcontainer.hxx>
 #include <comphelper/processfactory.hxx>
@@ -60,6 +61,21 @@ CPPUNIT_TEST_FIXTURE(OoxMathmlTest, testImportCharacters)
     CPPUNIT_ASSERT(getComponent().is());
 }
 
+CPPUNIT_TEST_FIXTURE(OoxMathmlTest, testImportMce)
+{
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "import-mce.pptx";
+    getComponent() = loadFromDesktop(aURL);
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+
+    // Without the accompanying fix in place, this failed with:
+    // - Expected: 1
+    // - Actual  : 2
+    // i.e. both the math object and its replacement image was imported, as two separate objects.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), xDrawPage->getCount());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/core/fragmenthandler2.cxx b/oox/source/core/fragmenthandler2.cxx
index 2d3f239966e0..79b58ce70f0e 100644
--- a/oox/source/core/fragmenthandler2.cxx
+++ b/oox/source/core/fragmenthandler2.cxx
@@ -26,6 +26,7 @@
 namespace oox {
 namespace core {
 
+using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::xml::sax;
 
@@ -72,14 +73,26 @@ bool FragmentHandler2::prepareMceContext( sal_Int32 nElement, const AttributeLis
                 // is long gone. For now let's decide depending on a list of supported
                 // namespaces like we do in writerfilter
 
-                static std::vector<OUString> aSupportedNS =
+                std::vector<OUString> aSupportedNS =
                 {
+                    "a14", // Impress needs this to import math formulas.
                     "p14",
                     "p15",
                     "x12ac",
                     "v",
                 };
 
+                uno::Reference<lang::XServiceInfo> xModel(getFilter().getModel(), uno::UNO_QUERY);
+                if (xModel.is() && xModel->supportsService("com.sun.star.sheet.SpreadsheetDocument"))
+                {
+                    // No a14 for Calc documents, it would cause duplicated shapes as-is.
+                    auto it = std::find(aSupportedNS.begin(), aSupportedNS.end(), "a14");
+                    if (it != aSupportedNS.end())
+                    {
+                        aSupportedNS.erase(it);
+                    }
+                }
+
                 if (std::find(aSupportedNS.begin(), aSupportedNS.end(), aRequires) != aSupportedNS.end())
                     aMceState.back() = MCE_STATE::FoundChoice;
                 else
diff --git a/oox/source/drawingml/shapegroupcontext.cxx b/oox/source/drawingml/shapegroupcontext.cxx
index 2bd12eafcb50..6d7756e86d5b 100644
--- a/oox/source/drawingml/shapegroupcontext.cxx
+++ b/oox/source/drawingml/shapegroupcontext.cxx
@@ -44,8 +44,8 @@ using namespace ::com::sun::star::xml::sax;
 
 namespace oox { namespace drawingml {
 
-ShapeGroupContext::ShapeGroupContext( ContextHandler2Helper const & rParent, ShapePtr const & pMasterShapePtr, ShapePtr const & pGroupShapePtr )
-: ContextHandler2( rParent )
+ShapeGroupContext::ShapeGroupContext( FragmentHandler2 const & rParent, ShapePtr const & pMasterShapePtr, ShapePtr const & pGroupShapePtr )
+: FragmentHandler2( rParent )
 , mpGroupShapePtr( pGroupShapePtr )
 {
     if( pMasterShapePtr )
diff --git a/oox/source/ppt/pptshapegroupcontext.cxx b/oox/source/ppt/pptshapegroupcontext.cxx
index 745a9b8e847e..251fff8ab78e 100644
--- a/oox/source/ppt/pptshapegroupcontext.cxx
+++ b/oox/source/ppt/pptshapegroupcontext.cxx
@@ -23,6 +23,7 @@
 #include <com/sun/star/container/XNamed.hpp>
 
 #include <oox/core/xmlfilterbase.hxx>
+#include <oox/core/fragmenthandler2.hxx>
 #include <oox/helper/attributelist.hxx>
 #include <oox/ppt/pptshape.hxx>
 #include <oox/ppt/pptgraphicshapecontext.hxx>
@@ -51,7 +52,7 @@ using namespace ::com::sun::star::xml::sax;
 namespace oox { namespace ppt {
 
 PPTShapeGroupContext::PPTShapeGroupContext(
-        ContextHandler2Helper const & rParent,
+        FragmentHandler2 const & rParent,
         const oox::ppt::SlidePersistPtr& rSlidePersistPtr,
         const ShapeLocation eShapeLocation,
         const oox::drawingml::ShapePtr& pMasterShapePtr,
diff --git a/oox/source/ppt/slidefragmenthandler.cxx b/oox/source/ppt/slidefragmenthandler.cxx
index 615665d8ae3c..3257c31bfdde 100644
--- a/oox/source/ppt/slidefragmenthandler.cxx
+++ b/oox/source/ppt/slidefragmenthandler.cxx
@@ -129,7 +129,6 @@ SlideFragmentHandler::~SlideFragmentHandler()
 
     case PPT_TOKEN( spTree ):           // CT_GroupShape
         {
-            // TODO Convert this to FragmentHandler2
             return new PPTShapeGroupContext(
                 *this, mpSlidePersistPtr, meShapeLocation, mpSlidePersistPtr->getShapes(),
                 oox::drawingml::ShapePtr( new PPTShape( meShapeLocation, "com.sun.star.drawing.GroupShape" ) ) );
diff --git a/oox/source/shape/LockedCanvasContext.cxx b/oox/source/shape/LockedCanvasContext.cxx
index 35d4aa62c65f..faf9644a1e37 100644
--- a/oox/source/shape/LockedCanvasContext.cxx
+++ b/oox/source/shape/LockedCanvasContext.cxx
@@ -23,8 +23,8 @@ namespace oox
 namespace shape
 {
 
-LockedCanvasContext::LockedCanvasContext(ContextHandler2Helper const& rParent)
-    : ContextHandler2(rParent)
+LockedCanvasContext::LockedCanvasContext(FragmentHandler2 const& rParent)
+    : FragmentHandler2(rParent)
 {
 }
 
diff --git a/oox/source/shape/LockedCanvasContext.hxx b/oox/source/shape/LockedCanvasContext.hxx
index 793491fe648c..e445e7a1a9cf 100644
--- a/oox/source/shape/LockedCanvasContext.hxx
+++ b/oox/source/shape/LockedCanvasContext.hxx
@@ -10,7 +10,7 @@
 #ifndef INCLUDED_OOX_SOURCE_SHAPE_LOCKEDCANVASCONTEXT_HXX
 #define INCLUDED_OOX_SOURCE_SHAPE_LOCKEDCANVASCONTEXT_HXX
 
-#include <oox/core/contexthandler2.hxx>
+#include <oox/core/fragmenthandler2.hxx>
 #include <oox/drawingml/drawingmltypes.hxx>
 
 namespace oox
@@ -19,10 +19,10 @@ namespace shape
 {
 
 /// Locked canvas is kind of a container for drawingml shapes: it can even contain group shapes.
-class LockedCanvasContext final : public oox::core::ContextHandler2
+class LockedCanvasContext final : public oox::core::FragmentHandler2
 {
 public:
-    explicit LockedCanvasContext(oox::core::ContextHandler2Helper const& rParent);
+    explicit LockedCanvasContext(oox::core::FragmentHandler2 const& rParent);
     ~LockedCanvasContext() override;
 
     oox::core::ContextHandlerRef onCreateContext(sal_Int32 nElementToken, const ::oox::AttributeList& rAttribs) override;
diff --git a/oox/source/shape/ShapeContextHandler.cxx b/oox/source/shape/ShapeContextHandler.cxx
index 1062589c91b6..23041d6cee48 100644
--- a/oox/source/shape/ShapeContextHandler.cxx
+++ b/oox/source/shape/ShapeContextHandler.cxx
@@ -68,7 +68,7 @@ uno::Reference<xml::sax::XFastContextHandler> const & ShapeContextHandler::getLo
         switch (nElement & 0xffff)
         {
             case XML_lockedCanvas:
-                mxLockedCanvasContext.set(new LockedCanvasContext(*rFragmentHandler));
+                mxLockedCanvasContext.set(static_cast<oox::core::ContextHandler*>(new LockedCanvasContext(*rFragmentHandler)));
                 break;
             default:
                 break;
@@ -145,7 +145,7 @@ uno::Reference<xml::sax::XFastContextHandler> const & ShapeContextHandler::getWp
         switch (getBaseToken(nElement))
         {
             case XML_wgp:
-                mxWpgContext.set(new WpgContext(*rFragmentHandler));
+                mxWpgContext.set(static_cast<oox::core::ContextHandler*>(new WpgContext(*rFragmentHandler)));
                 break;
             default:
                 break;
diff --git a/oox/source/shape/WpgContext.cxx b/oox/source/shape/WpgContext.cxx
index 88521e981ee6..36fb5cb976c0 100644
--- a/oox/source/shape/WpgContext.cxx
+++ b/oox/source/shape/WpgContext.cxx
@@ -23,8 +23,8 @@ namespace oox
 namespace shape
 {
 
-WpgContext::WpgContext(ContextHandler2Helper const& rParent)
-    : ContextHandler2(rParent)
+WpgContext::WpgContext(FragmentHandler2 const& rParent)
+    : FragmentHandler2(rParent)
 {
     mpShape.reset(new oox::drawingml::Shape("com.sun.star.drawing.GroupShape"));
     mpShape->setWps(true);
diff --git a/oox/source/shape/WpgContext.hxx b/oox/source/shape/WpgContext.hxx
index 1bdc53ecf09c..363750169040 100644
--- a/oox/source/shape/WpgContext.hxx
+++ b/oox/source/shape/WpgContext.hxx
@@ -10,7 +10,7 @@
 #ifndef INCLUDED_OOX_SOURCE_SHAPE_WPGCONTEXT_HXX
 #define INCLUDED_OOX_SOURCE_SHAPE_WPGCONTEXT_HXX
 
-#include <oox/core/contexthandler2.hxx>
+#include <oox/core/fragmenthandler2.hxx>
 #include <oox/drawingml/drawingmltypes.hxx>
 
 namespace oox
@@ -19,10 +19,10 @@ namespace shape
 {
 
 /// Wpg is the drawingML equivalent of v:group.
-class WpgContext final : public oox::core::ContextHandler2
+class WpgContext final : public oox::core::FragmentHandler2
 {
 public:
-    explicit WpgContext(oox::core::ContextHandler2Helper const& rParent);
+    explicit WpgContext(oox::core::FragmentHandler2 const& rParent);
     ~WpgContext() override;
 
     oox::core::ContextHandlerRef onCreateContext(sal_Int32 nElementToken, const oox::AttributeList& rAttribs) override;
diff --git a/sc/source/filter/inc/drawingfragment.hxx b/sc/source/filter/inc/drawingfragment.hxx
index 11d9b01a9f27..4fec8fe14367 100644
--- a/sc/source/filter/inc/drawingfragment.hxx
+++ b/sc/source/filter/inc/drawingfragment.hxx
@@ -75,14 +75,14 @@ class GroupShapeContext : public ::oox::drawingml::ShapeGroupContext, public Wor
 {
 public:
     explicit            GroupShapeContext(
-                            const ::oox::core::ContextHandler2Helper& rParent,
+                            const ::oox::core::FragmentHandler2& rParent,
                             const WorksheetHelper& rHelper,
                             const ::oox::drawingml::ShapePtr& rxParentShape,
                             const ::oox::drawingml::ShapePtr& rxShape );
 
     static ::oox::core::ContextHandlerRef
                         createShapeContext(
-                            ::oox::core::ContextHandler2Helper& rParent,
+                            ::oox::core::FragmentHandler2& rParent,
                             const WorksheetHelper& rHelper,
                             sal_Int32 nElement,
                             const AttributeList& rAttribs,
diff --git a/sc/source/filter/oox/drawingfragment.cxx b/sc/source/filter/oox/drawingfragment.cxx
index 0aa475a64f94..0eb6b460217d 100644
--- a/sc/source/filter/oox/drawingfragment.cxx
+++ b/sc/source/filter/oox/drawingfragment.cxx
@@ -121,14 +121,14 @@ void Shape::finalizeXShape( XmlFilterBase& rFilter, const Reference< XShapes >&
     }
 }
 
-GroupShapeContext::GroupShapeContext( const ContextHandler2Helper& rParent,
+GroupShapeContext::GroupShapeContext( const FragmentHandler2& rParent,
         const WorksheetHelper& rHelper, const ShapePtr& rxParentShape, const ShapePtr& rxShape ) :
     ShapeGroupContext( rParent, rxParentShape, rxShape ),
     WorksheetHelper( rHelper )
 {
 }
 
-/*static*/ ContextHandlerRef GroupShapeContext::createShapeContext( ContextHandler2Helper& rParent,
+/*static*/ ContextHandlerRef GroupShapeContext::createShapeContext( FragmentHandler2& rParent,
         const WorksheetHelper& rHelper, sal_Int32 nElement, const AttributeList& rAttribs,
         const ShapePtr& rxParentShape, ShapePtr* pxShape )
 {