From 4038d6c393c3cf6330671124ba69cdba98b24960 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 13 Nov 2019 17:41:45 +0100 Subject: tdf#117658 PPTX import: fix duplicated math object handling We used to recurse into both arms of : while the intention is that an importer either reads or . 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 Tested-by: Jenkins --- oox/qa/unit/data/import-mce.pptx | Bin 0 -> 33763 bytes oox/qa/unit/mathml.cxx | 18 +++++++++++++++++- oox/source/core/fragmenthandler2.cxx | 15 ++++++++++++++- oox/source/drawingml/shapegroupcontext.cxx | 4 ++-- oox/source/ppt/pptshapegroupcontext.cxx | 3 ++- oox/source/ppt/slidefragmenthandler.cxx | 1 - oox/source/shape/LockedCanvasContext.cxx | 4 ++-- oox/source/shape/LockedCanvasContext.hxx | 6 +++--- oox/source/shape/ShapeContextHandler.cxx | 4 ++-- oox/source/shape/WpgContext.cxx | 4 ++-- oox/source/shape/WpgContext.hxx | 6 +++--- 11 files changed, 47 insertions(+), 18 deletions(-) create mode 100644 oox/qa/unit/data/import-mce.pptx (limited to 'oox') 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 Binary files /dev/null and b/oox/qa/unit/data/import-mce.pptx differ 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 #include -#include +#include #include +#include #include #include @@ -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 xDrawPagesSupplier(getComponent(), uno::UNO_QUERY); + uno::Reference 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(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 aSupportedNS = + std::vector aSupportedNS = { + "a14", // Impress needs this to import math formulas. "p14", "p15", "x12ac", "v", }; + uno::Reference 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 #include +#include #include #include #include @@ -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 +#include #include 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 const & ShapeContextHandler::getLo switch (nElement & 0xffff) { case XML_lockedCanvas: - mxLockedCanvasContext.set(new LockedCanvasContext(*rFragmentHandler)); + mxLockedCanvasContext.set(static_cast(new LockedCanvasContext(*rFragmentHandler))); break; default: break; @@ -145,7 +145,7 @@ uno::Reference const & ShapeContextHandler::getWp switch (getBaseToken(nElement)) { case XML_wgp: - mxWpgContext.set(new WpgContext(*rFragmentHandler)); + mxWpgContext.set(static_cast(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 +#include #include 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; -- cgit