summaryrefslogtreecommitdiff
path: root/toolkit
diff options
context:
space:
mode:
authorMichael Weghorn <m.weghorn@posteo.de>2022-09-02 13:06:08 +0200
committerMichael Weghorn <m.weghorn@posteo.de>2022-09-02 15:47:37 +0200
commit206543c7bef58fc559852553a3b2faba0b604259 (patch)
tree4fc3e0e57e6f9b50e276998fc04241035a2dffb6 /toolkit
parent38671e21d7dbcd5019912b9468305018de0c922e (diff)
[API CHANGE] tdf#150683 a11y: Switch a11y child index to 64 bit
With 16k column support in Calc enabled by default in commit 4c5f8ccf0a2320432b8fe91add1dcadf54d9fd58 Date: Tue Mar 8 12:44:49 2022 +0100 change default Calc number of columns to 16384 (tdf#50916) , the number of Calc cells in a spreadsheet is larger than SAL_MAX_INT32, meaning that a 32-bit a11y child index is no more enough and using it resulted in integer overflows in methods handling corresponding Calc cells in the a11y layer. This e.g. had the effect of the Orca and NVDA screen readers not announcing focused or selected cells properly when their a11y child index was out of the 32-bit integer range. Switch the internal a11y child indices to 64 bit to be able to handle this properly internally. Since the platform APIs (at least AT-SPI on Linux and IAccessible2 on Windows; from what I can see LO's macOS a11y bridge doesn't directly expose the child index) are still restricted to 32 bit, larger child indices still cannot be exposed via the platform APIs. As a consequence, use of the the IAccessible2 and AT-SPI methods that use the child index remains problematic in those cases where the child index is larger. However, as an alternative to using the AT-SPI Table interface and the IAccessibleTable/ IAccessibleTable2 interfaces with the child index to retrieve information about a specific cell, both AT-SPI and IAccessible2 also provide interfaces to retrieve that information directly from the cell object (TableCell interface for AT-SPI, IAccessibleTableCell for IAccessible2). Those interfaces are already implemented/exposed for winaccessibility (s. `CAccTable`) and the qt5/qt6/kf5 VCL plugins (s. the `QAccessibleTableCellInterface` methods implemented in `QtAccessibleInterface`). With the switch to 64-bit internal a11y child indices, these now behave correctly for cells with a child index that doesn't fit into 32 bit as well. NVDA on Windows already uses the IAccessibleTableCell interface and thus announcing focused cells works fine with this change in place. Orca on Linux currently doesn't make use of the AT-SPI TableCell interface yet, but with a suggested change to do so [1], announcement of selected cells works with the qt6 VCL plugin with a current qtbase dev branch as well - when combined with the suggested changes to implement support for the AT-SPI TableCell interface in Qt [2] [3] and the LO change based on that [4] and a fix for a nullptr dereference [5]. The gtk3 VCL plugin doesn't expose the AT-SPI TableCell interface yet, but once it does so (via `AtkTableCell`), it also works with the suggested Orca change [1] in place. (Adding that is planned for an upcoming change, works with a local WIP patch.) For handling return values that are larger than what platform APIs support, the following approach has been chosen for now: 1) When the return value is for the count of (selected) children, the maximum value N supported by the platform API is returned. (This is what `ScAccessibleTableBase::getAccessibleChildCount` did previously.) The first N elements can be accessed by their actual (selection) indices. 2) When the return value is the child/cell index, -2 is returned for objects whose index is greater than the maximum value supported by the platform API. Using a non-negative value would mean that the index would refer to *another* actually existing child. A child index of -1 on the other hand tends to be interpreted as "child is invalid" or "object isn't actually a child of its (previous) parent any more)". For the Orca case, this would result in objects with a child index of -1 not being announced, as they are considered "zombies" [6]. What's still somewhat problematic is the case where more than 2^31 children are *selected*, since access to those children still happens by the index into the selection in the platform APIs, and not all selected items are accessible this way. (Screen readers usually just retrieve the first and last element from the selection and announce those.) Orca already seems to apply different handling for the case for fully selected rows and columns, so "All cells selected" or "Columns ... to ... selected" is announced just fine even if more than 2^31 cells are selected. (Side note: While Microsoft User Interface Automation - UIA - also uses 32-bit indices, it also has specific methods in the ISelectionProvider2 interface that allow to explicitly retrieve the first and last selected item, `ISelectionProvider2::get_FirstSelectedItem` and `ISelectionProvider2::get_LastSelectedItem`, but we currently don't support UIA on Windows.) Bound checks at the beginning of the methods from the `XAccessibleContext`, `XAccessibleSelection` and `XAccessibleTable` interfaces that take a child index (or in helper methods called by those) should generally already prevent too large indices from being passed to the methods in the lower layer code that take smaller integer types. Such bound checking has been been added in various places where it wasn't present yet. If there any remaining issues of this kind that show after this commit, they can probably be solved in a similar way (s.e.g. the change to `AccessibleBrowseBox::getAccessibleChild` in this commit). A few asserts were also added at places where my understanding is that values shouldn't be larger than what is supported by a called method anyway. A test case will be added in a following change. [1] https://gitlab.gnome.org/GNOME/orca/-/merge_requests/131 [2] https://codereview.qt-project.org/c/qt/qtbase/+/428566 [3] https://codereview.qt-project.org/c/qt/qtbase/+/428567 [4] https://gerrit.libreoffice.org/c/core/+/138750 [5] https://codereview.qt-project.org/c/qt/qtbase/+/430157 [6] https://gitlab.gnome.org/GNOME/orca/-/blob/82c8542002e36e0d3d918088d583162d25136143/src/orca/script_utilities.py#L5155 Change-Id: I3af590c988b0e6754fc72545918412f39e8fea07 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139258 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Diffstat (limited to 'toolkit')
-rw-r--r--toolkit/inc/controls/accessiblecontrolcontext.hxx4
-rw-r--r--toolkit/qa/complex/toolkit/AccessibleStatusBarItem.java4
-rw-r--r--toolkit/qa/complex/toolkit/accessibility/_XAccessibleComponent.java8
-rw-r--r--toolkit/qa/complex/toolkit/accessibility/_XAccessibleContext.java8
-rw-r--r--toolkit/qa/cppunit/a11y/XAccessibleComponentTester.cxx4
-rw-r--r--toolkit/qa/cppunit/a11y/XAccessibleContextTester.cxx10
-rw-r--r--toolkit/source/awt/vclxaccessiblecomponent.cxx16
-rw-r--r--toolkit/source/controls/accessiblecontrolcontext.cxx4
-rw-r--r--toolkit/test/accessibility/AccessibleCellHandler.java14
-rw-r--r--toolkit/test/accessibility/AccessibleSelectionHandler.java6
-rw-r--r--toolkit/test/accessibility/AccessibleTreeHandler.java2
11 files changed, 40 insertions, 40 deletions
diff --git a/toolkit/inc/controls/accessiblecontrolcontext.hxx b/toolkit/inc/controls/accessiblecontrolcontext.hxx
index ebe928218535..20065b86f101 100644
--- a/toolkit/inc/controls/accessiblecontrolcontext.hxx
+++ b/toolkit/inc/controls/accessiblecontrolcontext.hxx
@@ -68,8 +68,8 @@ namespace toolkit
private:
// XAccessibleContext
- virtual sal_Int32 SAL_CALL getAccessibleChildCount( ) override;
- virtual css::uno::Reference< css::accessibility::XAccessible > SAL_CALL getAccessibleChild( sal_Int32 i ) override;
+ virtual sal_Int64 SAL_CALL getAccessibleChildCount( ) override;
+ virtual css::uno::Reference< css::accessibility::XAccessible > SAL_CALL getAccessibleChild( sal_Int64 i ) override;
virtual css::uno::Reference< css::accessibility::XAccessible > SAL_CALL getAccessibleParent( ) override;
virtual sal_Int16 SAL_CALL getAccessibleRole( ) override;
virtual OUString SAL_CALL getAccessibleDescription( ) override;
diff --git a/toolkit/qa/complex/toolkit/AccessibleStatusBarItem.java b/toolkit/qa/complex/toolkit/AccessibleStatusBarItem.java
index a3fac8c3dec0..d2c9b0f68a43 100644
--- a/toolkit/qa/complex/toolkit/AccessibleStatusBarItem.java
+++ b/toolkit/qa/complex/toolkit/AccessibleStatusBarItem.java
@@ -250,10 +250,10 @@ public class AccessibleStatusBarItem {
}
public void runAllInterfaceTests() {
- int count = testObject.getAccessibleChildCount();
+ long count = testObject.getAccessibleChildCount();
System.out.println("*****");
System.out.println("**** Found items to test: " + count);
- for (int i=0;i<count;i++){
+ for (long i = 0; i < count; i++) {
System.out.println("**** Now testing StatusBarItem " + i + ".");
XAccessible object = null;
try {
diff --git a/toolkit/qa/complex/toolkit/accessibility/_XAccessibleComponent.java b/toolkit/qa/complex/toolkit/accessibility/_XAccessibleComponent.java
index 3d4df2c5ff19..e55cb1a22a1a 100644
--- a/toolkit/qa/complex/toolkit/accessibility/_XAccessibleComponent.java
+++ b/toolkit/qa/complex/toolkit/accessibility/_XAccessibleComponent.java
@@ -219,8 +219,8 @@ public class _XAccessibleComponent {
+ (chBnd.X ) + "," + chBnd.Y + ") - OK");
boolean res = util.AccessibilityTools.equals(xAccCh, xAcc);
if (!res) {
- int expIndex = xAccCh.getAccessibleContext().getAccessibleIndexInParent();
- int gotIndex = xAcc.getAccessibleContext().getAccessibleIndexInParent();
+ long expIndex = xAccCh.getAccessibleContext().getAccessibleIndexInParent();
+ long gotIndex = xAcc.getAccessibleContext().getAccessibleIndexInParent();
if (expIndex < gotIndex) {
System.out.println("The children found is not the same");
System.out.println("The expected child " +
@@ -387,7 +387,7 @@ public class _XAccessibleComponent {
return new XAccessibleComponent[0];
}
XAccessibleContext xAccCon = xAcc.getAccessibleContext();
- int cnt = xAccCon.getAccessibleChildCount();
+ long cnt = xAccCon.getAccessibleChildCount();
// for cases when too many children exist checking only first 50
if (cnt > 50)
@@ -396,7 +396,7 @@ public class _XAccessibleComponent {
}
ArrayList<XAccessibleComponent> childComp = new ArrayList<XAccessibleComponent>();
- for (int i = 0; i < cnt; i++) {
+ for (long i = 0; i < cnt; i++) {
try {
XAccessible child = xAccCon.getAccessibleChild(i);
XAccessibleContext xAccConCh = child.getAccessibleContext();
diff --git a/toolkit/qa/complex/toolkit/accessibility/_XAccessibleContext.java b/toolkit/qa/complex/toolkit/accessibility/_XAccessibleContext.java
index e52855efa01a..645b3b03b9ef 100644
--- a/toolkit/qa/complex/toolkit/accessibility/_XAccessibleContext.java
+++ b/toolkit/qa/complex/toolkit/accessibility/_XAccessibleContext.java
@@ -49,7 +49,7 @@ public class _XAccessibleContext {
private final XAccessibleContext oObj;
- private int childCount = 0;
+ private long childCount = 0;
private XAccessible parent = null ;
public _XAccessibleContext(XInterface object) {
@@ -80,12 +80,12 @@ public class _XAccessibleContext {
*/
public boolean _getAccessibleChild() {
boolean bOK = true;
- int counter = childCount;
+ long counter = childCount;
if (childCount > 500)
{
counter = 500;
}
- for (int i = 0; i < counter; i++) {
+ for (long i = 0; i < counter; i++) {
try {
XAccessible ch = oObj.getAccessibleChild(i) ;
XAccessibleContext chAC = ch.getAccessibleContext();
@@ -151,7 +151,7 @@ public class _XAccessibleContext {
public boolean _getAccessibleIndexInParent() {
boolean bOK = true;
- int idx = oObj.getAccessibleIndexInParent();
+ long idx = oObj.getAccessibleIndexInParent();
XAccessibleContext parentAC = parent.getAccessibleContext() ;
try {
diff --git a/toolkit/qa/cppunit/a11y/XAccessibleComponentTester.cxx b/toolkit/qa/cppunit/a11y/XAccessibleComponentTester.cxx
index 4e9ed7a1ee00..fa5fb9f7054f 100644
--- a/toolkit/qa/cppunit/a11y/XAccessibleComponentTester.cxx
+++ b/toolkit/qa/cppunit/a11y/XAccessibleComponentTester.cxx
@@ -138,9 +138,9 @@ void XAccessibleComponentTester::testContainsPoint()
*/
void XAccessibleComponentTester::testAccessibleAtPoint()
{
- int count = mxContext->getAccessibleChildCount();
+ sal_Int64 count = mxContext->getAccessibleChildCount();
std::cout << "Found " << count << " children" << std::endl;
- for (int i = 0; i < count && i < AccessibilityTools::MAX_CHILDREN; i++)
+ for (sal_Int64 i = 0; i < count && i < AccessibilityTools::MAX_CHILDREN; i++)
{
auto child = mxContext->getAccessibleChild(i);
uno::Reference<accessibility::XAccessibleContext> childContext(
diff --git a/toolkit/qa/cppunit/a11y/XAccessibleContextTester.cxx b/toolkit/qa/cppunit/a11y/XAccessibleContextTester.cxx
index e9b605c73956..9d7fdb992eae 100644
--- a/toolkit/qa/cppunit/a11y/XAccessibleContextTester.cxx
+++ b/toolkit/qa/cppunit/a11y/XAccessibleContextTester.cxx
@@ -33,8 +33,8 @@
*/
void XAccessibleContextTester::testGetAccessibleChild()
{
- int count = mxContext->getAccessibleChildCount();
- for (int i = 0; i < count && i < AccessibilityTools::MAX_CHILDREN; i++)
+ sal_Int64 count = mxContext->getAccessibleChildCount();
+ for (sal_Int64 i = 0; i < count && i < AccessibilityTools::MAX_CHILDREN; i++)
{
auto child = mxContext->getAccessibleChild(i);
auto childCtx = child->getAccessibleContext();
@@ -55,9 +55,9 @@ void XAccessibleContextTester::testGetAccessibleChild()
*/
void XAccessibleContextTester::testGetAccessibleChildCount()
{
- auto childCount = mxContext->getAccessibleChildCount();
+ sal_Int64 childCount = mxContext->getAccessibleChildCount();
std::cout << childCount << " children found." << std::endl;
- CPPUNIT_ASSERT_GREATEREQUAL(static_cast<sal_Int32>(0), childCount);
+ CPPUNIT_ASSERT_GREATEREQUAL(static_cast<sal_Int64>(0), childCount);
}
/**
@@ -80,7 +80,7 @@ void XAccessibleContextTester::testGetAccessibleDescription()
*/
void XAccessibleContextTester::testGetAccessibleIndexInParent()
{
- int idx = mxContext->getAccessibleIndexInParent();
+ sal_Int64 idx = mxContext->getAccessibleIndexInParent();
std::cout << "The index in parent is " << idx << std::endl;
auto parent = mxContext->getAccessibleParent();
diff --git a/toolkit/source/awt/vclxaccessiblecomponent.cxx b/toolkit/source/awt/vclxaccessiblecomponent.cxx
index b45b1084e7d8..b7c44f53bd68 100644
--- a/toolkit/source/awt/vclxaccessiblecomponent.cxx
+++ b/toolkit/source/awt/vclxaccessiblecomponent.cxx
@@ -491,18 +491,18 @@ TRANSIENT
// accessibility::XAccessibleContext
-sal_Int32 VCLXAccessibleComponent::getAccessibleChildCount()
+sal_Int64 VCLXAccessibleComponent::getAccessibleChildCount()
{
OExternalLockGuard aGuard( this );
- sal_Int32 nChildren = 0;
+ sal_Int64 nChildren = 0;
if ( GetWindow() )
nChildren = GetWindow()->GetAccessibleChildWindowCount();
return nChildren;
}
-uno::Reference< accessibility::XAccessible > VCLXAccessibleComponent::getAccessibleChild( sal_Int32 i )
+uno::Reference< accessibility::XAccessible > VCLXAccessibleComponent::getAccessibleChild( sal_Int64 i )
{
OExternalLockGuard aGuard( this );
@@ -542,11 +542,11 @@ uno::Reference< accessibility::XAccessible > VCLXAccessibleComponent::getAccessi
return xAcc;
}
-sal_Int32 VCLXAccessibleComponent::getAccessibleIndexInParent( )
+sal_Int64 VCLXAccessibleComponent::getAccessibleIndexInParent( )
{
OExternalLockGuard aGuard( this );
- sal_Int32 nIndex = -1;
+ sal_Int64 nIndex = -1;
if ( GetWindow() )
{
@@ -561,8 +561,8 @@ sal_Int32 VCLXAccessibleComponent::getAccessibleIndexInParent( )
uno::Reference< accessibility::XAccessibleContext > xParentContext ( xParentAcc->getAccessibleContext() );
if ( xParentContext.is() )
{
- sal_Int32 nChildCount = xParentContext->getAccessibleChildCount();
- for ( sal_Int32 i=0; i<nChildCount; i++ )
+ sal_Int64 nChildCount = xParentContext->getAccessibleChildCount();
+ for ( sal_Int64 i = 0; i < nChildCount; i++ )
{
uno::Reference< accessibility::XAccessible > xChild( xParentContext->getAccessibleChild(i) );
if ( xChild.is() )
@@ -664,7 +664,7 @@ uno::Reference< accessibility::XAccessible > VCLXAccessibleComponent::getAccessi
OExternalLockGuard aGuard( this );
uno::Reference< accessibility::XAccessible > xChild;
- for ( sal_uInt32 i = 0, nCount = getAccessibleChildCount(); i < nCount; ++i )
+ for ( sal_Int64 i = 0, nCount = getAccessibleChildCount(); i < nCount; ++i )
{
uno::Reference< accessibility::XAccessible > xAcc = getAccessibleChild( i );
if ( xAcc.is() )
diff --git a/toolkit/source/controls/accessiblecontrolcontext.cxx b/toolkit/source/controls/accessiblecontrolcontext.cxx
index 07b9e8e98018..fa6d43f0b374 100644
--- a/toolkit/source/controls/accessiblecontrolcontext.cxx
+++ b/toolkit/source/controls/accessiblecontrolcontext.cxx
@@ -126,14 +126,14 @@ namespace toolkit
}
- sal_Int32 SAL_CALL OAccessibleControlContext::getAccessibleChildCount( )
+ sal_Int64 SAL_CALL OAccessibleControlContext::getAccessibleChildCount( )
{
// we do not have children
return 0;
}
- Reference< XAccessible > SAL_CALL OAccessibleControlContext::getAccessibleChild( sal_Int32 )
+ Reference< XAccessible > SAL_CALL OAccessibleControlContext::getAccessibleChild( sal_Int64 )
{
// we do not have children
throw IndexOutOfBoundsException();
diff --git a/toolkit/test/accessibility/AccessibleCellHandler.java b/toolkit/test/accessibility/AccessibleCellHandler.java
index c251f679064c..716e1374ef4a 100644
--- a/toolkit/test/accessibility/AccessibleCellHandler.java
+++ b/toolkit/test/accessibility/AccessibleCellHandler.java
@@ -77,7 +77,7 @@ class AccessibleCellHandler extends NodeHandler
{
case 0:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nRow = xTable.getAccessibleRow( nChild );
aChild = new StringNode ("# table row: " + nRow, aParent);
@@ -85,7 +85,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 1:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nCol = xTable.getAccessibleColumn( nChild );
aChild = new StringNode ("# table column: " + nCol, aParent);
@@ -93,7 +93,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 2:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nRow = xTable.getAccessibleRow( nChild );
int nCol = xTable.getAccessibleColumn( nChild );
int nExt = xTable.getAccessibleRowExtentAt( nRow, nCol );
@@ -103,7 +103,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 3:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nRow = xTable.getAccessibleRow( nChild );
int nCol = xTable.getAccessibleColumn( nChild );
int nExt = xTable.getAccessibleColumnExtentAt( nRow, nCol );
@@ -113,7 +113,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 4:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nRow = xTable.getAccessibleRow( nChild );
int nCol = xTable.getAccessibleColumn( nChild );
XAccessible xChild =
@@ -135,7 +135,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 6:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nRow = xTable.getAccessibleRow( nChild );
boolean bSelected =
xTable.isAccessibleRowSelected( nRow );
@@ -145,7 +145,7 @@ class AccessibleCellHandler extends NodeHandler
break;
case 7:
{
- int nChild = xContext.getAccessibleIndexInParent();
+ long nChild = xContext.getAccessibleIndexInParent();
int nCol = xTable.getAccessibleColumn( nChild );
boolean bSelected =
xTable.isAccessibleColumnSelected( nCol );
diff --git a/toolkit/test/accessibility/AccessibleSelectionHandler.java b/toolkit/test/accessibility/AccessibleSelectionHandler.java
index 4a8cd24ba9c9..41eae0d980fa 100644
--- a/toolkit/test/accessibility/AccessibleSelectionHandler.java
+++ b/toolkit/test/accessibility/AccessibleSelectionHandler.java
@@ -72,12 +72,12 @@ class AccessibleSelectionHandler
{
VectorNode aVNode =
new VectorNode( "Selected Children", aParent);
- int nSelected = 0;
- int nCount = ((AccTreeNode)aParent).getContext().
+ long nSelected = 0;
+ long nCount = ((AccTreeNode)aParent).getContext().
getAccessibleChildCount();
try
{
- for( int i = 0; i < nCount; i++ )
+ for( long i = 0; i < nCount; i++ )
{
try
{
diff --git a/toolkit/test/accessibility/AccessibleTreeHandler.java b/toolkit/test/accessibility/AccessibleTreeHandler.java
index e022dbae8d6b..09fe7902517c 100644
--- a/toolkit/test/accessibility/AccessibleTreeHandler.java
+++ b/toolkit/test/accessibility/AccessibleTreeHandler.java
@@ -101,7 +101,7 @@ class AccessibleTreeHandler
XAccessibleContext xContext = xChild.getAccessibleContext();
if (xContext != null)
{
- int nIndex = xContext.getAccessibleIndexInParent() + 1;
+ long nIndex = xContext.getAccessibleIndexInParent() + 1;
synchronized (maChildList)
{
if ((nIndex >= 0) || (nIndex <= maChildList.size()))