On Wed, 9 Mar 2022 16:11:02 GMT, Anton Litvinov <alitvi...@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
