Date: prev next · Thread: first prev next last
2013 Archives by date, by thread · List index


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/3372

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/72/3372/1

resolved fdo#63421 crash in pivot table with accessibility

The scenario of fdo#63421 (loading data and re-dragging the same field)
is not needed, simple data is sufficient and crash happened also when
dragging (removing) a field from a pane and dropping it anywhere else.

Multiple errors:
* getAccessibleChildCount() must return the real current count of
  children, not what mpFieldWindow says; AtkListener::updateChildList()
  uses this value to repopulate its own list; a child is added after it
  is added to mpFieldWindow but removed before it is removed from
  mpFieldWindow;
* LostFocus() uses an index of -1 if the last child was already removed
  and the field was dropped after dragging it away from a pane, handle
  that but it still does not look right
* RemoveField() called CommitChange() with
  AccessibleEventObject::NewValue set instead of OldValue, leading to
  AtkListener::handleChildAdded() being called instead of
  handleChildRemoved()

Apparently this never worked since 2002.

Change-Id: Idfb59d947002d2212bc67b414daecb65c55edae8
(cherry picked from commit 26114dcdf9d55a5a2490de6de619337e9733b0e2)
---
M sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
1 file changed, 64 insertions(+), 27 deletions(-)



diff --git a/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx 
b/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
index 035004ae..8490572 100644
--- a/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
+++ b/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
@@ -259,7 +259,7 @@
         AccessibleEventObject aEvent;
         aEvent.EventId = AccessibleEventId::CHILD;
         aEvent.Source = uno::Reference< XAccessibleContext >(this);
-        aEvent.NewValue <<= xTempAcc;
+        aEvent.OldValue <<= xTempAcc;
 
         CommitChange(aEvent); // gone child - event
 
@@ -270,25 +270,41 @@
 
 void ScAccessibleDataPilotControl::FieldFocusChange(sal_Int32 nOldIndex, sal_Int32 nNewIndex)
 {
-    OSL_ENSURE(static_cast<size_t>(nOldIndex) < maChildren.size() &&
-                static_cast<size_t>(nNewIndex) < maChildren.size(), "did not recognize a child 
count change");
+    if (0 <= nOldIndex && static_cast<size_t>(nOldIndex) < maChildren.size())
+    {
+        uno::Reference < XAccessible > xTempAcc = maChildren[nOldIndex].xWeakAcc;
+        if (xTempAcc.is() && maChildren[nOldIndex].pAcc)
+            maChildren[nOldIndex].pAcc->ResetFocused();
+    }
+    else
+    {
+        SAL_WARN( "sc", "ScAccessibleDataPilotControl::FieldFocusChange() old index out of bounds: 
" << nOldIndex);
+    }
 
-    uno::Reference < XAccessible > xTempAcc = maChildren[nOldIndex].xWeakAcc;
-    if (xTempAcc.is() && maChildren[nOldIndex].pAcc)
-        maChildren[nOldIndex].pAcc->ResetFocused();
-
-    xTempAcc = maChildren[nNewIndex].xWeakAcc;
-    if (xTempAcc.is() && maChildren[nNewIndex].pAcc)
-        maChildren[nNewIndex].pAcc->SetFocused();
+    if (0 <= nNewIndex && static_cast<size_t>(nNewIndex) < maChildren.size())
+    {
+        uno::Reference < XAccessible > xTempAcc = maChildren[nNewIndex].xWeakAcc;
+        if (xTempAcc.is() && maChildren[nNewIndex].pAcc)
+            maChildren[nNewIndex].pAcc->SetFocused();
+    }
+    else
+    {
+        SAL_WARN( "sc", "ScAccessibleDataPilotControl::FieldFocusChange() new index out of bounds: 
" << nNewIndex);
+    }
 }
 
 void ScAccessibleDataPilotControl::FieldNameChange(sal_Int32 nIndex)
 {
-    OSL_ENSURE(static_cast<size_t>(nIndex) < maChildren.size(), "did not recognize a child count 
change");
-
-    uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
-    if (xTempAcc.is() && maChildren[nIndex].pAcc)
-        maChildren[nIndex].pAcc->ChangeName();
+    if (0 <= nIndex && static_cast<size_t>(nIndex) < maChildren.size())
+    {
+        uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
+        if (xTempAcc.is() && maChildren[nIndex].pAcc)
+            maChildren[nIndex].pAcc->ChangeName();
+    }
+    else
+    {
+        SAL_WARN( "sc", "ScAccessibleDataPilotControl::FieldNameChange() index out of bounds: " << 
nIndex);
+    }
 }
 
 void ScAccessibleDataPilotControl::GotFocus()
@@ -298,9 +314,16 @@
         OSL_ENSURE(mpFieldWindow->GetFieldCount() == maChildren.size(), "did not recognize a child 
count change");
 
         sal_Int32 nIndex(mpFieldWindow->GetSelectedField());
-        uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
-        if (xTempAcc.is() && maChildren[nIndex].pAcc)
-            maChildren[nIndex].pAcc->SetFocused();
+        if (0 <= nIndex && static_cast<size_t>(nIndex) < maChildren.size())
+        {
+            uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
+            if (xTempAcc.is() && maChildren[nIndex].pAcc)
+                maChildren[nIndex].pAcc->SetFocused();
+        }
+        else
+        {
+            SAL_WARN( "sc", "ScAccessibleDataPilotControl::GotFocus() field index out of bounds: " 
<< nIndex);
+        }
     }
 }
 
@@ -311,9 +334,21 @@
         OSL_ENSURE(mpFieldWindow->GetFieldCount() == maChildren.size(), "did not recognize a child 
count change");
 
         sal_Int32 nIndex(mpFieldWindow->GetSelectedField());
-        uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
-        if (xTempAcc.is() && maChildren[nIndex].pAcc)
-            maChildren[nIndex].pAcc->ResetFocused();
+        if (0 <= nIndex && static_cast<size_t>(nIndex) < maChildren.size())
+        {
+            uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
+            if (xTempAcc.is() && maChildren[nIndex].pAcc)
+                maChildren[nIndex].pAcc->ResetFocused();
+        }
+        else
+        {
+            // This may actually happen if the last field is dropped somewhere
+            // and was already removed from the pane by dragging it away. This
+            // is odd.. looks like all LostFocus() are called for the previous
+            // field of the same original pane in the remove case?
+            SAL_WARN_IF( nIndex != -1 || !maChildren.empty(), "sc",
+                    "ScAccessibleDataPilotControl::LostFocus() field index out of bounds: " << 
nIndex);
+        }
     }
 }
 
@@ -390,10 +425,7 @@
 {
     SolarMutexGuard aGuard;
     IsObjectValid();
-    if (mpFieldWindow)
-        return mpFieldWindow->GetFieldCount();
-    else
-        return 0;
+    return static_cast<sal_Int32>(maChildren.size());
 }
 
 uno::Reference< XAccessible> SAL_CALL ScAccessibleDataPilotControl::getAccessibleChild(sal_Int32 
nIndex)
@@ -404,14 +436,19 @@
     uno::Reference<XAccessible> xAcc;
     if (mpFieldWindow)
     {
-        if (nIndex < 0 || static_cast< size_t >( nIndex ) >= mpFieldWindow->GetFieldCount())
+        if (nIndex < 0 || static_cast< size_t >( nIndex ) >= maChildren.size())
             throw lang::IndexOutOfBoundsException();
 
-        OSL_ENSURE(mpFieldWindow->GetFieldCount() == maChildren.size(), "did not recognize a child 
count change");
+        OSL_ENSURE( mpFieldWindow->GetFieldCount() == maChildren.size()         // all except ...
+                ||  mpFieldWindow->GetFieldCount() == maChildren.size() + 1,    // in CommitChange 
during RemoveField
+                "did not recognize a child count change");
 
         uno::Reference < XAccessible > xTempAcc = maChildren[nIndex].xWeakAcc;
         if (!xTempAcc.is())
         {
+            if (static_cast< size_t >( nIndex ) >= mpFieldWindow->GetFieldCount())
+                throw lang::IndexOutOfBoundsException();
+
             maChildren[nIndex].pAcc = new ScAccessibleDataPilotButton(this, mpFieldWindow, nIndex);
             xTempAcc = maChildren[nIndex].pAcc;
             maChildren[nIndex].xWeakAcc = xTempAcc;

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfb59d947002d2212bc67b414daecb65c55edae8
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Eike Rathke <erack@redhat.com>


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.