From f05a551a6b49916241696481567e32b320c6749b Mon Sep 17 00:00:00 2001 From: Armin Le Grand Date: Wed, 31 Oct 2018 20:47:49 +0100 Subject: tdf#120728 support SvxShape for SdrShape if not inserted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit 52bbb04f1e39b2d778275c91f77b6c0714ecd0d0) Reviewed-on: https://gerrit.libreoffice.org/62738 Tested-by: Xisco Faulí Reviewed-by: Thorsten Behrens --- svx/source/svdraw/svdobj.cxx | 57 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) (limited to 'svx/source') diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx index 5b313c907358..7300dbe2f5b8 100644 --- a/svx/source/svdraw/svdobj.cxx +++ b/svx/source/svdraw/svdobj.cxx @@ -2815,9 +2815,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); @@ -2831,6 +2880,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 ); } -- cgit