[PATCH libreoffice-3-6] resolved fdo#63421 crash in pivot table with accessibility
Eike Rathke (via Code Review)
gerrit at gerrit.libreoffice.org
Sat Apr 13 07:39:59 PDT 2013
Hi,
I have submitted a patch for review:
https://gerrit.libreoffice.org/3374
To pull it, you can do:
git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/74/3374/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.
(cherry picked from commit 26114dcdf9d55a5a2490de6de619337e9733b0e2)
Conflicts:
sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
Change-Id: Idfb59d947002d2212bc67b414daecb65c55edae8
---
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 0b826fa..c71eff6 100644
--- a/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
+++ b/sc/source/ui/Accessibility/AccessibleDataPilotControl.cxx
@@ -268,7 +268,7 @@
AccessibleEventObject aEvent;
aEvent.EventId = AccessibleEventId::CHILD;
aEvent.Source = uno::Reference< XAccessibleContext >(this);
- aEvent.NewValue <<= xTempAcc;
+ aEvent.OldValue <<= xTempAcc;
CommitChange(aEvent); // gone child - event
@@ -279,25 +279,41 @@
void ScAccessibleDataPilotControl::FieldFocusChange(sal_Int32 nOldIndex, sal_Int32 nNewIndex)
{
- OSL_ENSURE(static_cast<sal_uInt32>(nOldIndex) < maChildren.size() &&
- static_cast<sal_uInt32>(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<sal_uInt32>(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()
@@ -307,9 +323,16 @@
OSL_ENSURE(static_cast<sal_uInt32>(mpDPFieldWindow->GetFieldCount()) == maChildren.size(), "did not recognize a child count change");
sal_Int32 nIndex(mpDPFieldWindow->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);
+ }
}
}
@@ -320,9 +343,21 @@
OSL_ENSURE(static_cast<sal_uInt32>(mpDPFieldWindow->GetFieldCount()) == maChildren.size(), "did not recognize a child count change");
sal_Int32 nIndex(mpDPFieldWindow->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);
+ }
}
}
@@ -405,10 +440,7 @@
{
SolarMutexGuard aGuard;
IsObjectValid();
- if (mpDPFieldWindow)
- return mpDPFieldWindow->GetFieldCount();
- else
- return 0;
+ return static_cast<sal_Int32>(maChildren.size());
}
uno::Reference< XAccessible> SAL_CALL ScAccessibleDataPilotControl::getAccessibleChild(sal_Int32 nIndex)
@@ -419,14 +451,19 @@
uno::Reference<XAccessible> xAcc;
if (mpDPFieldWindow)
{
- if (nIndex < 0 || static_cast< size_t >( nIndex ) >= mpDPFieldWindow->GetFieldCount())
+ if (nIndex < 0 || static_cast< size_t >( nIndex ) >= maChildren.size())
throw lang::IndexOutOfBoundsException();
- OSL_ENSURE(static_cast<sal_uInt32>(mpDPFieldWindow->GetFieldCount()) == maChildren.size(), "did not recognize a child count change");
+ OSL_ENSURE( mpDPFieldWindow->GetFieldCount() == maChildren.size() // all except ...
+ || mpDPFieldWindow->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 ) >= mpDPFieldWindow->GetFieldCount())
+ throw lang::IndexOutOfBoundsException();
+
maChildren[nIndex].pAcc = new ScAccessibleDataPilotButton(this, mpDPFieldWindow, nIndex);
xTempAcc = maChildren[nIndex].pAcc;
maChildren[nIndex].xWeakAcc = xTempAcc;
--
To view, visit https://gerrit.libreoffice.org/3374
To unsubscribe, visit https://gerrit.libreoffice.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfb59d947002d2212bc67b414daecb65c55edae8
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-3-6
Gerrit-Owner: Eike Rathke <erack at redhat.com>
More information about the LibreOffice
mailing list