Skip to content

Commit

Permalink
8275819: [TableRowAccessibility accessibilityChildren] method is inef…
Browse files Browse the repository at this point in the history
…fective

Reviewed-by: pbansal, kizune
  • Loading branch information
Anton Tarasov committed Oct 26, 2021
1 parent 71d593e commit b98ed55
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.Arrays;
import java.util.function.Function;

import javax.accessibility.Accessible;
import javax.accessibility.AccessibleAction;
Expand Down Expand Up @@ -662,21 +663,29 @@ public boolean[] call() throws Exception {
@Native static final int JAVA_AX_SELECTED_CHILDREN = -2;
@Native static final int JAVA_AX_VISIBLE_CHILDREN = -3;

private static Object[] getTableRowChildrenAndRoles(Accessible a, Component c, int whichChildren, boolean allowIgnored, int tableRowIndex) {
return invokeGetChildrenAndRoles(a, c, whichChildren, allowIgnored, ChildrenOperations.createForTableRow(tableRowIndex));
}

// Each child takes up two entries in the array: one for itself and one for its role
public static Object[] getChildrenAndRoles(final Accessible a, final Component c, final int whichChildren, final boolean allowIgnored) {
private static Object[] getChildrenAndRoles(final Accessible a, final Component c, final int whichChildren, final boolean allowIgnored) {
return invokeGetChildrenAndRoles(a, c, whichChildren, allowIgnored, ChildrenOperations.COMMON);
}

private static Object[] invokeGetChildrenAndRoles(Accessible a, Component c, int whichChildren, boolean allowIgnored, ChildrenOperations ops) {
if (a == null) return null;
return invokeAndWait(new Callable<Object[]>() {
public Object[] call() throws Exception {
return getChildrenAndRolesImpl(a, c, whichChildren, allowIgnored);
return getChildrenAndRolesImpl(a, c, whichChildren, allowIgnored, ops);
}
}, c);
}

private static Object[] getChildrenAndRolesImpl(final Accessible a, final Component c, final int whichChildren, final boolean allowIgnored) {
private static Object[] getChildrenAndRolesImpl(Accessible a, Component c, int whichChildren, boolean allowIgnored, ChildrenOperations ops) {
if (a == null) return null;

ArrayList<Object> childrenAndRoles = new ArrayList<Object>();
_addChildren(a, whichChildren, allowIgnored, childrenAndRoles);
_addChildren(a, whichChildren, allowIgnored, childrenAndRoles, ops);

/* In case of fetching a selection, we need to check if
* the active descendant is at the beginning of the list, or
Expand Down Expand Up @@ -747,7 +756,7 @@ public Object[] call() throws Exception {
while (!parentStack.isEmpty()) {
Accessible p = parentStack.get(parentStack.size() - 1);

currentLevelChildren.addAll(Arrays.asList(getChildrenAndRolesImpl(p, c, JAVA_AX_ALL_CHILDREN, allowIgnored)));
currentLevelChildren.addAll(Arrays.asList(getChildrenAndRolesImpl(p, c, JAVA_AX_ALL_CHILDREN, allowIgnored, ChildrenOperations.COMMON)));
if ((currentLevelChildren.size() == 0) || (index >= currentLevelChildren.size())) {
if (!parentStack.isEmpty()) parentStack.remove(parentStack.size() - 1);
if (!indexses.isEmpty()) index = indexses.remove(indexses.size() - 1);
Expand Down Expand Up @@ -859,20 +868,70 @@ private static AccessibleRole getAccessibleRole(Accessible a) {
return role;
}

private interface ChildrenOperations {
boolean isContextValid(AccessibleContext accessibleContext);
int getChildrenCount(AccessibleContext accessibleContext);
Accessible getAccessibleChild(AccessibleContext accessibleContext, int childIndex);

static ChildrenOperations COMMON = createForCommon();

static ChildrenOperations createForCommon() {
return new ChildrenOperations() {
@Override
public boolean isContextValid(AccessibleContext accessibleContext) {
return accessibleContext != null;
}

@Override
public int getChildrenCount(AccessibleContext accessibleContext) {
assert isContextValid(accessibleContext);
return accessibleContext.getAccessibleChildrenCount();
}

@Override
public Accessible getAccessibleChild(AccessibleContext accessibleContext, int childIndex) {
assert isContextValid(accessibleContext);
return accessibleContext.getAccessibleChild(childIndex);
}
};
}

static ChildrenOperations createForTableRow(int tableRowIndex) {
return new ChildrenOperations() {
@Override
public boolean isContextValid(AccessibleContext accessibleContext) {
return accessibleContext instanceof AccessibleTable;
}

@Override
public int getChildrenCount(AccessibleContext accessibleContext) {
assert isContextValid(accessibleContext);
return ((AccessibleTable)accessibleContext).getAccessibleColumnCount();
}

@Override
public Accessible getAccessibleChild(AccessibleContext accessibleContext, int childIndex) {
assert isContextValid(accessibleContext);
return ((AccessibleTable)accessibleContext).getAccessibleAt(tableRowIndex, childIndex);
}
};
}
}


// Either gets the immediate children of a, or recursively gets all unignored children of a
private static void _addChildren(final Accessible a, final int whichChildren, final boolean allowIgnored, final ArrayList<Object> childrenAndRoles) {
private static void _addChildren(Accessible a, int whichChildren, boolean allowIgnored, ArrayList<Object> childrenAndRoles, ChildrenOperations ops) {
if (a == null) return;

final AccessibleContext ac = a.getAccessibleContext();
if (ac == null) return;
if (!ops.isContextValid(ac)) return;

final int numChildren = ac.getAccessibleChildrenCount();
final int numChildren = ops.getChildrenCount(ac);

// each child takes up two entries in the array: itself, and its role
// so the array holds alternating Accessible and AccessibleRole objects
for (int i = 0; i < numChildren; i++) {
final Accessible child = ac.getAccessibleChild(i);
final Accessible child = ops.getAccessibleChild(ac, i);
if (child == null) continue;

final AccessibleContext context = child.getAccessibleContext();
Expand All @@ -894,7 +953,7 @@ private static void _addChildren(final Accessible a, final int whichChildren, fi
final AccessibleRole role = context.getAccessibleRole();
if (role != null && ignoredRoles != null && ignoredRoles.contains(roleKey(role))) {
// Get the child's unignored children.
_addChildren(child, whichChildren, false, childrenAndRoles);
_addChildren(child, whichChildren, false, childrenAndRoles, ChildrenOperations.COMMON);
} else {
childrenAndRoles.add(child);
childrenAndRoles.add(getAccessibleRole(child));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@

static jclass sjc_CAccessibility = NULL;

static jmethodID jm_getChildrenAndRoles = NULL;
#define GET_CHILDRENANDROLES_METHOD_RETURN(ret) \
GET_CACCESSIBILITY_CLASS_RETURN(ret); \
GET_STATIC_METHOD_RETURN(jm_getChildrenAndRoles, sjc_CAccessibility, "getChildrenAndRoles",\
"(Ljavax/accessibility/Accessible;Ljava/awt/Component;IZ)[Ljava/lang/Object;", ret);

@implementation TableRowAccessibility

// NSAccessibilityElement protocol methods
Expand All @@ -55,25 +49,28 @@ - (NSAccessibilitySubrole)accessibilitySubrole

- (NSArray *)accessibilityChildren
{
NSArray *children = [super accessibilityChildren];
NSMutableArray *children = [super accessibilityChildren];
if (children == nil) {
JNIEnv *env = [ThreadUtilities getJNIEnv];
CommonComponentAccessibility *parent = [self accessibilityParent];
if (parent->fAccessible == NULL) return nil;
GET_CHILDRENANDROLES_METHOD_RETURN(nil);
jobjectArray jchildrenAndRoles = (jobjectArray)(*env)->CallStaticObjectMethod(env, sjc_CAccessibility, jm_getChildrenAndRoles,
parent->fAccessible, parent->fComponent, sun_lwawt_macosx_CAccessibility_JAVA_AX_ALL_CHILDREN, NO);

GET_CACCESSIBILITY_CLASS_RETURN(nil);
DECLARE_STATIC_METHOD_RETURN(jm_getTableRowChildrenAndRoles, sjc_CAccessibility, "getTableRowChildrenAndRoles",\
"(Ljavax/accessibility/Accessible;Ljava/awt/Component;IZI)[Ljava/lang/Object;", nil);

jobjectArray jchildrenAndRoles = (jobjectArray)(*env)->CallStaticObjectMethod(
env, sjc_CAccessibility, jm_getTableRowChildrenAndRoles, parent->fAccessible, parent->fComponent,
sun_lwawt_macosx_CAccessibility_JAVA_AX_ALL_CHILDREN, NO, [self rowNumberInTable]);
CHECK_EXCEPTION();

if (jchildrenAndRoles == NULL) return nil;

jsize arrayLen = (*env)->GetArrayLength(env, jchildrenAndRoles);
NSMutableArray *childrenCells = [NSMutableArray arrayWithCapacity:arrayLen/2];
children = [NSMutableArray arrayWithCapacity:arrayLen / 2];
int childIndex = [self rowNumberInTable] * [(TableAccessibility *)parent accessibilityColumnCount];

NSUInteger childIndex = fIndex * [(TableAccessibility *)parent accessibilityColumnCount];
NSInteger i = childIndex * 2;
NSInteger n = (fIndex + 1) * [(TableAccessibility *)parent accessibilityColumnCount] * 2;
for(i; i < n; i+=2)
{
for (NSInteger i = 0; i < arrayLen; i += 2) {
jobject /* Accessible */ jchild = (*env)->GetObjectArrayElement(env, jchildrenAndRoles, i);
jobject /* String */ jchildJavaRole = (*env)->GetObjectArrayElement(env, jchildrenAndRoles, i+1);

Expand All @@ -93,18 +90,20 @@ - (NSArray *)accessibilityChildren
withIndex:childIndex
withView:self->fView
withJavaRole:childJavaRole];
[childrenCells addObject:[[child retain] autorelease]];
[children addObject:[[child retain] autorelease]];

(*env)->DeleteLocalRef(env, jchild);
(*env)->DeleteLocalRef(env, jchildJavaRole);

childIndex++;
}
(*env)->DeleteLocalRef(env, jchildrenAndRoles);
return childrenCells;
} else {
return children;
}
return children;
}

- (NSUInteger)rowNumberInTable {
return self->fIndex;
}

- (NSInteger)accessibilityIndex
Expand Down

3 comments on commit b98ed55

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forantar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on b98ed55 Nov 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forantar the backport was successfully created on the branch forantar-backport-b98ed550 in my personal fork of openjdk/jdk17u. To create a pull request with this backport targeting openjdk/jdk17u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit b98ed550 from the openjdk/jdk repository.

The commit being backported was authored by Anton Tarasov on 26 Oct 2021 and was reviewed by Pankaj Bansal and Alexander Zuev.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u:

$ git fetch https://github.com/openjdk-bots/jdk17u forantar-backport-b98ed550:forantar-backport-b98ed550
$ git checkout forantar-backport-b98ed550
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u forantar-backport-b98ed550

Please sign in to comment.