Make ActiveX controls import working again (PPTX / XLSX)

It used to work earlier, but there were an issue with the
shape id and so controls were not find. Also in PPTX import
the persistStorage attribute was handled only for parent
controls and not for other kind of controls.

Change-Id: I9784166b65407b79b6dfed8a38087b55b1b69835
Reviewed-on: https://gerrit.libreoffice.org/40751
Reviewed-by: Tamás Zolnai <tamas.zolnai@collabora.com>
Tested-by: Tamás Zolnai <tamas.zolnai@collabora.com>

9 files changed, 98 insertions, 15 deletions

diff --git a/include/oox/vml/vmldrawing.hxx b/include/oox/vml/vmldrawing.hxx
index e9309f6c8497..032d29494a44 100644
--- a/include/oox/vml/vmldrawing.hxx
+++ b/include/oox/vml/vmldrawing.hxx
@@ -78,14 +78,10 @@ struct OOX_DLLPUBLIC OleObjectInfo : public ::oox::ole::OleObjectInfo
 /** Contains information about a form control embedded in a draw page. */
 struct OOX_DLLPUBLIC ControlInfo
 {
-    OUString     maShapeId;          ///< Shape identifier for shape lookup.
     OUString     maFragmentPath;     ///< Path to the fragment describing the form control properties.
     OUString     maName;             ///< Programmatical name of the form control.
 
     explicit            ControlInfo();
-
-    /** Sets the string representation of the passed numeric shape identifier. */
-    void                setShapeId( sal_Int32 nShapeId );
 };
 
 
diff --git a/oox/source/ole/axcontrolfragment.cxx b/oox/source/ole/axcontrolfragment.cxx
index fb8a4652c01b..351a4bd2aec7 100644
--- a/oox/source/ole/axcontrolfragment.cxx
+++ b/oox/source/ole/axcontrolfragment.cxx
@@ -130,11 +130,30 @@ ContextHandlerRef AxControlFragment::onCreateContext( sal_Int32 nElement, const
                     Reference< XInputStream > xStrgStrm = getFilter().openInputStream( aFragmentPath );
                     if( xStrgStrm.is() )
                     {
+                        // Try to import as a parent control
+                        bool bImportedAsParent = false;
                         OleStorage aStorage( getFilter().getComponentContext(), xStrgStrm, false );
                         BinaryXInputStream aInStrm( aStorage.openInputStream( "f" ), true );
                         if( !aInStrm.isEof() )
+                        {
                             if( AxContainerModelBase* pModel = dynamic_cast< AxContainerModelBase* >( mrControl.createModelFromGuid( aClassId ) ) )
+                            {
                                 pModel->importBinaryModel( aInStrm );
+                                bImportedAsParent = true;
+                            }
+                        }
+                        // Import it as a non-parent control
+                        if(!bImportedAsParent)
+                        {
+                            BinaryXInputStream aInStrm2(aStorage.openInputStream("contents"), true);
+                            if (!aInStrm2.isEof())
+                            {
+                                if (ControlModelBase* pModel = mrControl.createModelFromGuid(aClassId))
+                                {
+                                    pModel->importBinaryModel(aInStrm2);
+                                }
+                            }
+                        }
                     }
                 }
             }
diff --git a/oox/source/ppt/slidefragmenthandler.cxx b/oox/source/ppt/slidefragmenthandler.cxx
index 9d6fcf16aa99..3a967eea14ce 100644
--- a/oox/source/ppt/slidefragmenthandler.cxx
+++ b/oox/source/ppt/slidefragmenthandler.cxx
@@ -139,7 +139,6 @@ SlideFragmentHandler::~SlideFragmentHandler()
     case PPT_TOKEN( control ):
         {
             ::oox::vml::ControlInfo aInfo;
-            aInfo.setShapeId( rAttribs.getInteger( XML_spid, 0 ) );
             aInfo.maFragmentPath = getFragmentPathFromRelId( rAttribs.getString( R_TOKEN( id ), OUString() ) );
             aInfo.maName = rAttribs.getXString( XML_name, OUString() );
             mpSlidePersistPtr->getDrawing()->registerControl( aInfo );
diff --git a/oox/source/vml/vmldrawing.cxx b/oox/source/vml/vmldrawing.cxx
index 34d798b62d10..6cb8f4dc7ebe 100644
--- a/oox/source/vml/vmldrawing.cxx
+++ b/oox/source/vml/vmldrawing.cxx
@@ -82,11 +82,6 @@ ControlInfo::ControlInfo()
 {
 }
 
-void ControlInfo::setShapeId( sal_Int32 nShapeId )
-{
-    maShapeId = lclGetShapeId( nShapeId );
-}
-
 Drawing::Drawing( XmlFilterBase& rFilter, const Reference< XDrawPage >& rxDrawPage, DrawingType eType ) :
     mrFilter( rFilter ),
     mxDrawPage( rxDrawPage ),
@@ -129,10 +124,9 @@ void Drawing::registerOleObject( const OleObjectInfo& rOleObject )
 
 void Drawing::registerControl( const ControlInfo& rControl )
 {
-    OSL_ENSURE( !rControl.maShapeId.isEmpty(), "Drawing::registerControl - missing form control shape id" );
     OSL_ENSURE( !rControl.maName.isEmpty(), "Drawing::registerControl - missing form control name" );
-    OSL_ENSURE( maControls.count( rControl.maShapeId ) == 0, "Drawing::registerControl - form control already registered" );
-    maControls.insert( ControlInfoMap::value_type( rControl.maShapeId, rControl ) );
+    OSL_ENSURE( maControls.count( rControl.maName ) == 0, "Drawing::registerControl - form control already registered" );
+    maControls.insert( ControlInfoMap::value_type( rControl.maName, rControl ) );
 }
 
 void Drawing::finalizeFragmentImport()
diff --git a/sc/qa/unit/data/xlsx/activex_checkbox.xlsx b/sc/qa/unit/data/xlsx/activex_checkbox.xlsx
new file mode 100755
index 000000000000..b37bf59948aa
--- /dev/null
+++ b/sc/qa/unit/data/xlsx/activex_checkbox.xlsx
diff --git a/sc/qa/unit/subsequent_filters-test.cxx b/sc/qa/unit/subsequent_filters-test.cxx
index 131ae7bd1c73..0e838a87496e 100644
--- a/sc/qa/unit/subsequent_filters-test.cxx
+++ b/sc/qa/unit/subsequent_filters-test.cxx
@@ -241,6 +241,7 @@ public:
     void testTdf97598XLSX();
 
     void testPageScalingXLSX();
+    void testActiveXCheckboxXLSX();
 #ifdef UNX
     void testUnicodeFileNameGnumeric();
 #endif
@@ -366,6 +367,7 @@ public:
     CPPUNIT_TEST(testTdf97598XLSX);
 
     CPPUNIT_TEST(testPageScalingXLSX);
+    CPPUNIT_TEST(testActiveXCheckboxXLSX);
 #ifdef UNX
     CPPUNIT_TEST(testUnicodeFileNameGnumeric);
 #endif
@@ -3934,6 +3936,44 @@ void ScFiltersTest::testPageScalingXLSX()
     xDocSh->DoClose();
 }
 
+void ScFiltersTest::testActiveXCheckboxXLSX()
+{
+    ScDocShellRef xDocSh = loadDoc("activex_checkbox.", FORMAT_XLSX);
+    uno::Reference< frame::XModel > xModel = xDocSh->GetModel();
+    uno::Reference< sheet::XSpreadsheetDocument > xDoc(xModel, UNO_QUERY_THROW);
+    uno::Reference< container::XIndexAccess > xIA(xDoc->getSheets(), UNO_QUERY_THROW);
+    uno::Reference< drawing::XDrawPageSupplier > xDrawPageSupplier(xIA->getByIndex(0), UNO_QUERY_THROW);
+    uno::Reference< container::XIndexAccess > xIA_DrawPage(xDrawPageSupplier->getDrawPage(), UNO_QUERY_THROW);
+    uno::Reference< drawing::XControlShape > xControlShape(xIA_DrawPage->getByIndex(0), UNO_QUERY_THROW);
+    CPPUNIT_ASSERT(xControlShape.is());
+
+    // Check control type
+    uno::Reference<beans::XPropertySet> xPropertySet(xControlShape->getControl(), uno::UNO_QUERY);
+    uno::Reference<lang::XServiceInfo> xServiceInfo(xPropertySet, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(true, bool(xServiceInfo->supportsService("com.sun.star.form.component.CheckBox")));
+
+    // Check custom label
+    OUString sLabel;
+    xPropertySet->getPropertyValue("Label") >>= sLabel;
+    CPPUNIT_ASSERT_EQUAL(OUString("Custom Caption"), sLabel);
+
+    // Check background color (highlight system color)
+    sal_Int32 nColor;
+    xPropertySet->getPropertyValue("BackgroundColor") >>= nColor;
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0x316AC5), nColor);
+
+    // Check Text color (active border system color)
+    xPropertySet->getPropertyValue("TextColor") >>= nColor;
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD4D0C8), nColor);
+
+    // Check state of the checkbox
+    sal_Int16 nState;
+    xPropertySet->getPropertyValue("State") >>= nState;
+    CPPUNIT_ASSERT_EQUAL(sal_Int16(1), nState);
+
+    xDocSh->DoClose();
+}
+
 ScFiltersTest::ScFiltersTest()
       : ScBootstrapFixture( "sc/qa/unit/data" )
 {
diff --git a/sc/source/filter/oox/worksheetfragment.cxx b/sc/source/filter/oox/worksheetfragment.cxx
index 703593747c22..b5d593a46ec4 100644
--- a/sc/source/filter/oox/worksheetfragment.cxx
+++ b/sc/source/filter/oox/worksheetfragment.cxx
@@ -752,7 +752,6 @@ void WorksheetFragment::importOleObject( const AttributeList& rAttribs )
 void WorksheetFragment::importControl( const AttributeList& rAttribs )
 {
     ::oox::vml::ControlInfo aInfo;
-    aInfo.setShapeId( rAttribs.getInteger( XML_shapeId, 0 ) );
     aInfo.maFragmentPath = getFragmentPathFromRelId( rAttribs.getString( R_TOKEN( id ), OUString() ) );
     aInfo.maName = rAttribs.getString( XML_name, OUString() );
     getVmlDrawing().registerControl( aInfo );
@@ -889,7 +888,6 @@ void WorksheetFragment::importOleObject( SequenceInputStream& rStrm )
 void WorksheetFragment::importControl( SequenceInputStream& rStrm )
 {
     ::oox::vml::ControlInfo aInfo;
-    aInfo.setShapeId( rStrm.readInt32() );
     aInfo.maFragmentPath = getFragmentPathFromRelId( BiffHelper::readString( rStrm ) );
     rStrm >> aInfo.maName;
     getVmlDrawing().registerControl( aInfo );
diff --git a/sd/qa/unit/data/pptx/activex_checkbox.pptx b/sd/qa/unit/data/pptx/activex_checkbox.pptx
new file mode 100755
index 000000000000..66eac985b203
--- /dev/null
+++ b/sd/qa/unit/data/pptx/activex_checkbox.pptx
diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx
index 33d4a5a711f8..40804586e9db 100644
--- a/sd/qa/unit/import-tests.cxx
+++ b/sd/qa/unit/import-tests.cxx
@@ -70,6 +70,7 @@
 #include <com/sun/star/style/XStyleFamiliesSupplier.hpp>
 #include <com/sun/star/table/XTableRows.hpp>
 #include <com/sun/star/style/NumberingType.hpp>
+#include <com/sun/star/drawing/XControlShape.hpp>
 
 #include <stlpool.hxx>
 #include <comphelper/processfactory.hxx>
@@ -164,6 +165,7 @@ public:
     void testTdf109067();
     void testSmartArt1();
     void testTdf109223();
+    void testActiveXCheckbox();
 
     bool checkPattern(sd::DrawDocShellRef const & rDocRef, int nShapeNumber, std::vector<sal_uInt8>& rExpected);
     void testPatternImport();
@@ -236,6 +238,7 @@ public:
     CPPUNIT_TEST(testTdf109067);
     CPPUNIT_TEST(testSmartArt1);
     CPPUNIT_TEST(testTdf109223);
+    CPPUNIT_TEST(testActiveXCheckbox);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -2288,6 +2291,40 @@ void SdImportTest::testTdf109223()
     xDocShRef->DoClose();
 }
 
+void SdImportTest::testActiveXCheckbox()
+{
+    // ActiveX controls were imported as images
+    sd::DrawDocShellRef xDocShRef = loadURL(m_directories.getURLFromSrc("sd/qa/unit/data/pptx/activex_checkbox.pptx"), PPTX);
+    uno::Reference< drawing::XControlShape > xControlShape(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT(xControlShape.is());
+
+    // Check control type
+    uno::Reference<beans::XPropertySet> xPropertySet(xControlShape->getControl(), uno::UNO_QUERY);
+    uno::Reference<lang::XServiceInfo> xServiceInfo(xPropertySet, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(true, bool(xServiceInfo->supportsService("com.sun.star.form.component.CheckBox")));
+
+    // Check custom label
+    OUString sLabel;
+    xPropertySet->getPropertyValue("Label") >>= sLabel;
+    CPPUNIT_ASSERT_EQUAL(OUString("Custom Caption"), sLabel);
+
+    // Check background color (highlight system color)
+    sal_Int32 nColor;
+    xPropertySet->getPropertyValue("BackgroundColor") >>= nColor;
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0x316AC5), nColor);
+
+    // Check Text color (active border system color)
+    xPropertySet->getPropertyValue("TextColor") >>= nColor;
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD4D0C8), nColor);
+
+    // Check state of the checkbox
+    sal_Int16 nState;
+    xPropertySet->getPropertyValue("State") >>= nState;
+    CPPUNIT_ASSERT_EQUAL(sal_Int16(1), nState);
+
+    xDocShRef->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();