[Libreoffice-commits] core.git: 2 commits - sw/inc sw/source

Michael Stahl mstahl at redhat.com
Mon Dec 19 12:03:57 UTC 2016


 sw/inc/accmap.hxx                |    2 -
 sw/source/core/access/accmap.cxx |   41 ++++++++++--------------------
 sw/source/core/draw/dcontact.cxx |   52 +++++++++++++++------------------------
 3 files changed, 36 insertions(+), 59 deletions(-)

New commits:
commit b052b5d890be70dd22b9aea36a356074a2c39871
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon Dec 19 12:53:30 2016 +0100

    tdf#104488 SwAccessibleMap: dispose sub-shapes of group shapes
    
    ... and ensure that they are removed from the mpShapeMap.
    
    Commit 76c549eb01dcb7b5bf28a271ce00e386f3d388ba added all sub-shapes of
    group shapes to SwAccessibleMap::mpShapeMap.
    It's not immediately obvious why this is necessary, given that
    AccessibleShape will probably create these on demand.
    
    It turns out that the AccessibleShapes are not only disposed from
    SwAccessible code but also from svx code,
    in particular the call to ViewForwarderChanged() in
    SwAccessibleContext::ChildrenScrolled() leads to visibility checks.
    
    Thus the RemoveGroupContext() may fail to remove the sub-shapes from the
    mpShapeMap and the assertion in ~SwAccessibleMap is triggered.
    
    Change-Id: I6d9c3d816f5521960855d5e6a563bec49d8030b8

diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index 469fba1..aaef5a7 100644
--- a/sw/inc/accmap.hxx
+++ b/sw/inc/accmap.hxx
@@ -175,7 +175,7 @@ public:
 
     void AddGroupContext(const SdrObject *pParentObj,
                     css::uno::Reference < css::accessibility::XAccessible > const & xAccParent);
-    void RemoveGroupContext(const SdrObject *pParentObj, css::uno::Reference < css::accessibility::XAccessible > const & xAccParent);
+    void RemoveGroupContext(const SdrObject *pParentObj);
 
     const SwRect& GetVisArea() const;
 
diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx
index d2090e0..be1054b 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -2056,36 +2056,23 @@ void SwAccessibleMap::AddShapeContext(const SdrObject *pObj, uno::Reference < XA
 }
 
 //Added by yanjun for sym2_6407
-void SwAccessibleMap::RemoveGroupContext(const SdrObject *pParentObj, css::uno::Reference < css::accessibility::XAccessible > const & xAccParent)
+void SwAccessibleMap::RemoveGroupContext(const SdrObject *pParentObj)
 {
     osl::MutexGuard aGuard( maMutex );
-    if (mpShapeMap && pParentObj && pParentObj->IsGroupObject() && xAccParent.is())
+    // TODO: Why are sub-shapes of group shapes even added to our map?
+    //       Doesn't the AccessibleShape of the top-level shape create them
+    //       on demand anyway? Why does SwAccessibleMap need to know them?
+    // We cannot rely on getAccessibleChild here to remove the sub-shapes
+    // from mpShapes because the top-level shape may not only be disposed here
+    // but also by visibility checks in svx, then it doesn't return children.
+    if (mpShapeMap && pParentObj && pParentObj->IsGroupObject())
     {
-        uno::Reference < XAccessibleContext > xContext = xAccParent->getAccessibleContext();
-        if (xContext.is())
+        SdrObjList *const pChildren(pParentObj->GetSubList());
+        for (size_t i = 0; pChildren && i < pChildren->GetObjCount(); ++i)
         {
-            for (sal_Int32 i = 0; i < xContext->getAccessibleChildCount(); ++i)
-            {
-                uno::Reference < XAccessible > xChild = xContext->getAccessibleChild(i);
-                if (xChild.is())
-                {
-                    uno::Reference < XAccessibleContext > xChildContext = xChild->getAccessibleContext();
-                    if (xChildContext.is())
-                    {
-                        if (xChildContext->getAccessibleRole() == AccessibleRole::SHAPE)
-                        {
-                            ::accessibility::AccessibleShape* pAccShape = static_cast < ::accessibility::AccessibleShape* >( xChild.get());
-                            uno::Reference < drawing::XShape > xShape = pAccShape->GetXShape();
-                            if (xShape.is())
-                            {
-                                SdrObject* pObj = GetSdrObjectFromXShape(xShape);
-                                if (pObj)
-                                    RemoveContext(pObj);
-                            }
-                        }
-                    }
-                }
-            }
+            SdrObject *const pChild(pChildren->GetObj(i));
+            assert(pChild);
+            RemoveContext(pChild);
         }
     }
 }
@@ -2195,7 +2182,7 @@ void SwAccessibleMap::RemoveContext( const SdrObject *pObj )
         {
             uno::Reference < XAccessible > xAcc( (*aIter).second );
             mpShapeMap->erase( aIter );
-            RemoveGroupContext(pObj, xAcc);
+            RemoveGroupContext(pObj);
             // The shape selection flag is not cleared, but one might do
             // so but has to make sure that the removed context is the one
             // that is selected.
commit f256fafd7ab69e929d5267a487a23024a902897e
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Dec 16 22:30:37 2016 +0100

    sw: Sw*Contact::GetAnchoredObj() convert to assert and simplify
    
    Change-Id: I0aa18fafd1759cd7c4d2ada18120fa8b97b353a4

diff --git a/sw/source/core/draw/dcontact.cxx b/sw/source/core/draw/dcontact.cxx
index a1651f6..b3b5ad1 100644
--- a/sw/source/core/draw/dcontact.cxx
+++ b/sw/source/core/draw/dcontact.cxx
@@ -400,21 +400,15 @@ SwFlyDrawContact::~SwFlyDrawContact()
 }
 
 // #i26791#
-const SwAnchoredObject* SwFlyDrawContact::GetAnchoredObj( const SdrObject* _pSdrObj ) const
+const SwAnchoredObject* SwFlyDrawContact::GetAnchoredObj(const SdrObject* pSdrObj) const
 {
-    OSL_ENSURE( _pSdrObj,
-            "<SwFlyDrawContact::GetAnchoredObj(..)> - no object provided" );
-    OSL_ENSURE( dynamic_cast<const SwVirtFlyDrawObj*>( _pSdrObj) !=  nullptr,
-            "<SwFlyDrawContact::GetAnchoredObj(..)> - wrong object type object provided" );
-    assert(GetUserCall(_pSdrObj) == this &&
+    assert(pSdrObj);
+    assert(dynamic_cast<const SwVirtFlyDrawObj*>(pSdrObj) != nullptr);
+    assert(GetUserCall(pSdrObj) == this &&
         "<SwFlyDrawContact::GetAnchoredObj(..)> - provided object doesn't belong to this contact");
 
-    const SwAnchoredObject* pRetAnchoredObj = nullptr;
-
-    if (const SwVirtFlyDrawObj* pFlyDrawObj = dynamic_cast<const SwVirtFlyDrawObj*>(_pSdrObj))
-    {
-        pRetAnchoredObj = pFlyDrawObj->GetFlyFrame();
-    }
+    const SwAnchoredObject *const pRetAnchoredObj =
+        static_cast<const SwVirtFlyDrawObj*>(pSdrObj)->GetFlyFrame();
 
     return pRetAnchoredObj;
 }
@@ -642,35 +636,31 @@ void SwDrawContact::GetTextObjectsFromFormat( std::list<SdrTextObj*>& rTextObjec
 }
 
 // #i26791#
-const SwAnchoredObject* SwDrawContact::GetAnchoredObj( const SdrObject* _pSdrObj ) const
+const SwAnchoredObject* SwDrawContact::GetAnchoredObj(const SdrObject* pSdrObj ) const
 {
     // handle default parameter value
-    if ( !_pSdrObj )
+    if (!pSdrObj)
     {
-        _pSdrObj = GetMaster();
+        pSdrObj = GetMaster();
     }
 
-    OSL_ENSURE( _pSdrObj,
-            "<SwDrawContact::GetAnchoredObj(..)> - no object provided" );
-    OSL_ENSURE( dynamic_cast<const SwDrawVirtObj*>( _pSdrObj) !=  nullptr ||
-            ( dynamic_cast<const SdrVirtObj*>( _pSdrObj) == nullptr && dynamic_cast<const SwDrawVirtObj*>( _pSdrObj) == nullptr ),
-            "<SwDrawContact::GetAnchoredObj(..)> - wrong object type object provided" );
-    OSL_ENSURE( GetUserCall( _pSdrObj ) == this ||
-            _pSdrObj == GetMaster(),
+    assert(pSdrObj);
+    assert(dynamic_cast<const SwDrawVirtObj*>(pSdrObj) != nullptr ||
+           dynamic_cast<const SdrVirtObj*>(pSdrObj) == nullptr);
+    assert((GetUserCall(pSdrObj) == this ||
+            pSdrObj == GetMaster()) &&
             "<SwDrawContact::GetAnchoredObj(..)> - provided object doesn't belongs to this contact" );
 
     const SwAnchoredObject* pRetAnchoredObj = nullptr;
 
-    if ( _pSdrObj )
+    if (dynamic_cast<const SwDrawVirtObj*>(pSdrObj) != nullptr)
     {
-        if ( dynamic_cast<const SwDrawVirtObj*>( _pSdrObj) !=  nullptr )
-        {
-            pRetAnchoredObj = &(static_cast<const SwDrawVirtObj*>(_pSdrObj)->GetAnchoredObj());
-        }
-        else if ( dynamic_cast<const SdrVirtObj*>( _pSdrObj) == nullptr && dynamic_cast<const SwDrawVirtObj*>( _pSdrObj) == nullptr)
-        {
-            pRetAnchoredObj = &maAnchoredDrawObj;
-        }
+        pRetAnchoredObj = &(static_cast<const SwDrawVirtObj*>(pSdrObj)->GetAnchoredObj());
+    }
+    else
+    {
+        assert(dynamic_cast<const SdrVirtObj*>(pSdrObj) == nullptr);
+        pRetAnchoredObj = &maAnchoredDrawObj;
     }
 
     return pRetAnchoredObj;


More information about the Libreoffice-commits mailing list