svx/source/svdraw/svdobj.cxx |   57 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

New commits:
commit 52bbb04f1e39b2d778275c91f77b6c0714ecd0d0
Author:     Armin Le Grand <armin.le.gr...@cib.de>
AuthorDate: Wed Oct 31 20:47:49 2018 +0100
Commit:     Armin Le Grand <armin.le.gr...@cib.de>
CommitDate: Thu Nov 1 10:58:57 2018 +0100

    tdf#120728 support SvxShape for SdrShape if not inserted
    
    Added a fallback to allow correct SvxShape construction
    when SdrObject is not yet inserted - or has no SdrPage yet.
    This is needed ue to the paradigm change that a SdrObject
    only has a SdrPage when it is inserted.
    For more info and a discussion, see added comments.
    
    Change-Id: I2c1a4b1bbb531501ee73ab1c98c13321c5c0b050
    Reviewed-on: https://gerrit.libreoffice.org/62710
    Tested-by: Jenkins
    Reviewed-by: Armin Le Grand <armin.le.gr...@cib.de>

diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx
index 4df3c375243f..ebd1784361e6 100644
--- a/svx/source/svdraw/svdobj.cxx
+++ b/svx/source/svdraw/svdobj.cxx
@@ -2793,9 +2793,58 @@ css::uno::Reference< css::uno::XInterface > 
SdrObject::getUnoShape()
     if( !xShape.is() )
     {
         OSL_ENSURE( mpSvxShape == nullptr, "SdrObject::getUnoShape: XShape 
already dead, but still an IMPL pointer!" );
-        if ( getSdrPageFromSdrObject() )
+
+        // try to access SdrPage from this SdrObject. This will only exist if 
the SdrObject is
+        // inserted in a SdrObjList (page/group/3dScene)
+        SdrPage* pPageCandidate(getSdrPageFromSdrObject());
+
+        // tdf#12152, tdf#120728
+        //
+        // With the paradigm change to only get a SdrPage for a SdrObject when 
the SdrObject
+        // is *inserted*, the functionality for creating 1:1 associated UNO 
API implementation
+        // SvxShapes was partially broken: The used ::CreateShape relies on 
the SvxPage being
+        // derived and the CreateShape method overloaded, implementing 
additional SdrInventor
+        // types as needed.
+        //
+        // The fallback to use SvxDrawPage::CreateShapeByTypeAndInventor is a 
trap: It's only
+        // a static fallback that handles the SdrInventor types 
SdrInventor::E3d and
+        // SdrInventor::Default. Due to that, e.g. the ReportDesigner broke in 
various conditions.
+        //
+        // That again has to do with the ReportDesigner being implemented 
using the UNO API
+        // aspects of SdrObjects early during their construction, not just 
after these are
+        // inserted to a SdrPage - but that is not illegal or wrong, the 
SdrObject exists already.
+        //
+        // As a current solution, use the (now always available) SdrModel and 
any of the
+        // existing SdrPages. The only important thing is to get a SdrPage 
where ::CreateShape is
+        // overloaded and implemented as needed.
+        //
+        // Note for the future:
+        // In a more ideal world there would be only one factory method for 
creating SdrObjects (not
+        // ::CreateShape and ::CreateShapeByTypeAndInventor). This also would 
not be placed at
+        // SdrPage/SvxPage at all, but at the Model where it belongs - where 
else would you expect
+        // objects for the current Model to be constructed? To have this at 
the Page only would make
+        // sense if different shapes would need to be constructed for 
different Pages in the same Model
+        // - this is never the case.
+        // At that Model extended functionality for that factory (or overloads 
and implementations)
+        // should be placed. But to be realistic, migrating the factories to 
Model now is too much
+        // work - maybe over time when melting SdrObject/SvxObject one day...
+        if(nullptr == pPageCandidate)
+        {
+            // If not inserted, alternatively access a SdrPage using the 
SdrModel. There is
+            // no reason not to create and return a UNO API XShape when the 
SdrObject is not
+            // inserted - it may be in construction. Main paradigm is that it 
exists.
+            if(0 != getSdrModelFromSdrObject().GetPageCount())
+            {
+                // Take 1st SdrPage. That may be e.g. a special page (in SD), 
but the
+                // to-be-used method ::CreateShape will be correctly 
overloaded in
+                // all cases
+                pPageCandidate = getSdrModelFromSdrObject().GetPage(0);
+            }
+        }
+
+        if(nullptr != pPageCandidate)
         {
-            uno::Reference< uno::XInterface > xPage( 
getSdrPageFromSdrObject()->getUnoPage() );
+            uno::Reference< uno::XInterface > 
xPage(pPageCandidate->getUnoPage());
             if( xPage.is() )
             {
                 SvxDrawPage* pDrawPage = SvxDrawPage::getImplementation(xPage);
@@ -2809,6 +2858,10 @@ css::uno::Reference< css::uno::XInterface > 
SdrObject::getUnoShape()
         }
         else
         {
+            // Fallback to static base functionality. CAUTION: This will only 
support
+            // the most basic stuff like SdrInventor::E3d and 
SdrInventor::Default. All
+            // the other SdrInventor enum entries are from overloads and are 
*not accessible*
+            // using this fallback (!) - what a bad trap
             mpSvxShape = SvxDrawPage::CreateShapeByTypeAndInventor( 
GetObjIdentifier(), GetObjInventor(), this );
             maWeakUnoShape = xShape = static_cast< ::cppu::OWeakObject* >( 
mpSvxShape );
         }
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to