RFR: 8277922: Unable to click JCheckBox in JTable through Java Access Bridge [v3]

[Alexey Ivanov]

On Wed, 9 Mar 2022 16:11:02 GMT, Anton Litvinov <alitvinov at openjdk.org> wrote:

>> Hello,
>> 
>> Could you please review the following fix for the bug. The bug consists in the fact that When an assistive technology software by means of Java Access Bridge API executes "AccessibleAction" "click" on "AccessibleContext" which corresponds to "javax.swing.JTable" cell containing "javax.swing.JCheckBox", then the cell's value and cell's view represented by "JCheckBox" stay unchanged. The issue is a bug in JDK and should be fixed in JDK, because JDK informs the assistive technology software through Java Access Bridge API in particular through the function
>> 
>> "BOOL getAccessibleActions(long vmID, AccessibleContext accessibleContext, AccessibleActions *actions)"
>> 
>> that "AccessibleContext" of the table cell with "JCheckBox" supports one action "click", while real execution of this action on this accessible context does not lead to any result.
>> 
>> THE ROOT CAUSE OF THE BUG:
>> 
>> The reason of the issue is the fact that when the assistive technology software tries to do "AccessibleAction" on "AccessibleContext" associated with a cell with boolean data type in "JTable" component through Java Access Bridge (JAB), the JDK executes this "AccessibleAction" on "AccessibleContext" of a renderer, which is an instance of the class "javax.swing.JTable.BooleanRenderer" which is a derivative of "JCheckBox" class, and the instance of this renderer is single and common for all cells of boolean data type. Therefore execution of "click" "AccessibleAction" on this renderer component which is not permanently bound to any particular cell in the table does not lead to update of the table cell value.
>> 
>> THE FIX:
>> 
>> The fix implements an approach which guarantees setting of new values to the table's cells with boolean data type on each execution of "AccessibleAction" of "javax.swing.JTable.BooleanRenderer" instance, when execution of this action changes the "selected" state of this "BooleanRenderer" JCheckBox component.
>> 
>> Please take into account that the created automatic regression test simulates the issue only with Java Accessibility API what is not fully equal to the original test scenario which requires the assistive technology software and usage of Java Access Bridge API and which can be tested using the manual test case attached to the issue in JBS. However the regression test still allows to reproduce the issue and verify that the fix resolves it.
>> 
>> Thank you,
>> Anton
>
> Anton Litvinov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   The third version of the fix for JDK-8277922

The new approach by returning no accessible action looks cleaner. It resolves the ambiguity where the user of the API could get the accessible context of the cell renderer directly, in which case the accessible action cannot be performed because there's no way for the renderer to determine to which cell the action applies.

src/java.desktop/share/classes/javax/swing/JTable.java line 8417:

> 8415:                 if (ac != null) {
> 8416:                     return ac.getAccessibleAction();
> 8417:                 }

This is the fix for possible NPE, right?

test/jdk/javax/accessibility/JTable/BooleanRendererHasAccessibleActionTest.java line 48:

> 46: import javax.swing.table.TableCellRenderer;
> 47: 
> 48: public class BooleanRendererHasAccessibleActionTest {

Would `BooleanRendererHasNoAccessibleActionTest` be a better name? After all, you test for *no* accessible action.

test/jdk/javax/accessibility/JTable/BooleanRendererHasAccessibleActionTest.java line 50:

> 48: public class BooleanRendererHasAccessibleActionTest {
> 49:     private volatile JFrame frame;
> 50:     private volatile JTable table;

Neither `frame` nor `table` are accessed from any other thread but EDT, I believe `volatile` isn't necessary in this case.

test/jdk/javax/accessibility/JTable/BooleanRendererHasAccessibleActionTest.java line 142:

> 140: 
> 141:     private void testAccessibleActionInCellRenderer(int row, int column,
> 142:             boolean shouldBeNull) {

Since `shouldBeNull` is always `true`, I propose to drop it.

test/jdk/javax/accessibility/JTable/BooleanRendererHasAccessibleActionTest.java line 143:

> 141:     private void testAccessibleActionInCellRenderer(int row, int column,
> 142:             boolean shouldBeNull) {
> 143:         System.out.println(String.format(

I suggest using `System.out.printf`, just add `"\n"` to the end of format string.

test/jdk/javax/accessibility/JTable/BooleanRendererHasAccessibleActionTest.java line 161:

> 159:         AccessibleAction cellRendererAa = cellRendererAc.getAccessibleAction();
> 160:         if ((shouldBeNull && (cellRendererAa != null)) ||
> 161:             (!shouldBeNull && (cellRendererAa == null))) {

If `shouldBeNull` is dropped from parameters, this condition can be simplified to
Suggestion:

        if (cellRendererAa != null)) {


And the error message below should say _'cellRendererAa' is not null_.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7416