[PATCH libreoffice-4-0] SwPageFrm::RemoveFlyFromPage: fix a11y problem:

[Michael Stahl (via Code Review)]

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/2027

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/27/2027/1

SwPageFrm::RemoveFlyFromPage: fix a11y problem:

This problem can be reproduced with the bugdoc from fdo#60234, by
dragging and dropping the lower left drawing object.
The ATK bridge is notified of the change, and updates its list of
children by calling getAccessibleChildCount() and then
getAccessibleChild() in a loop.  Unfortunately the count returned does
not match the available children (the difference is the object that is
being dragged).  The implementations in
SwAccessibleFrame::GetChildCount() and SwAccessibleFrame::GetChild()
use slightly different iterators for the SwPageFrm, and the
SwAccessibleChildMap one is implemented as a std::map, using the OrdNum
of the drawing object as a key.  On the dragged object, that OrdNum has
been reset to 0 in RemoveFlyFromPage, but it is still in the pSortedObjs
of the SwPageFrm, and so it is not inserted into the map, and iteration
over the map is missing one element.

149     return _SwAccessibleChildMap::insert( aEntry );
Value returned is $22 = {first = {first = {eLayerId =
SwAccessibleChildMapKey::HELL, nOrdNum = 0}, second = ...}, second = false}

Fix this by removing the object from pSortedObjs before disposing
the a11y frames.

Change-Id: I4e4bb37759382e32150cff6def02d80a55e4a702
(cherry picked from commit 6bef95d0da47edf33f4c3fe3c7380977f8c1cf67)
---
M sw/source/core/layout/flylay.cxx
1 file changed, 17 insertions(+), 11 deletions(-)



diff --git a/sw/source/core/layout/flylay.cxx b/sw/source/core/layout/flylay.cxx
index 5d8f77b..6f7a283 100644
--- a/sw/source/core/layout/flylay.cxx
+++ b/sw/source/core/layout/flylay.cxx
@@ -701,6 +701,23 @@
     if ( pToRemove->IsFlyInCntFrm() )
         return;
 
+    // Don't delete collections just yet. This will happen at the end of the
+    // action in the RemoveSuperfluous of the page, kicked off by a method of
+    // the same name in the root.
+    // The FlyColl might be gone already, because the page's dtor is being
+    // executed.
+    // Remove it _before_ disposing accessible frames to avoid accesses to
+    // the Frm from event handlers.
+    if (pSortedObjs)
+    {
+        pSortedObjs->Remove(*pToRemove);
+        if (!pSortedObjs->Count())
+        {
+            delete pSortedObjs;
+            pSortedObjs = 0;
+        }
+    }
+
     // Notify accessible layout. That's required at this place for
     // frames only where the anchor is moved. Creation of new frames
     // is additionally handled by the SwFrmNotify class.
@@ -712,17 +729,6 @@
                                   ->DisposeAccessibleFrm( pToRemove, sal_True );
     }
 
-    // Don't delete collections just yet. This will happen at the end of the
-    // action in the RemoveSuperfluous of the page, kicked off by a method of
-    // the same name in the root.
-    // The FlyColl might be gone already, because the page's dtor is being executed.
-    if ( pSortedObjs )
-    {
-        pSortedObjs->Remove( *pToRemove );
-        if ( !pSortedObjs->Count() )
-        {   DELETEZ( pSortedObjs );
-        }
-    }
     // #i28701# - use new method <SetPageFrm(..)>
     pToRemove->SetPageFrm( 0L );
 }

-- 
To view, visit https://gerrit.libreoffice.org/2027
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4e4bb37759382e32150cff6def02d80a55e4a702
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Michael Stahl <mstahl at redhat.com>