[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - sw/inc sw/source

[Michael Stahl]

 sw/inc/accmap.hxx                |    2 -
 sw/source/core/access/accmap.cxx |   41 +++++++++++++--------------------------
 2 files changed, 15 insertions(+), 28 deletions(-)

New commits:
commit 211b9d9ef2b5c68a32c88453c9ae0e1131460eb8
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
    (cherry picked from commit b052b5d890be70dd22b9aea36a356074a2c39871)
    Reviewed-on: https://gerrit.libreoffice.org/32190
    Reviewed-by: Michael Stahl <mstahl at redhat.com>
    Tested-by: Michael Stahl <mstahl at redhat.com>

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 0aed777..2f23331 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -2066,36 +2066,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);
         }
     }
 }
@@ -2205,7 +2192,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.