<Swing Dev> [13] Review Request: 8226653 [accessibility] Can edit text cell correctly, but Accessibility Tool reads nothing about editor

[Philip Race]

OK +1 from me.

-phil.

On 7/11/19, 2:47 PM, Sergey Bylokhov wrote:
> On 11/07/2019 11:31, Phil Race wrote:
>> It seems we don't have a method like boolean 
>> Toolkit.isModifierKeyCode(int keyCode) ? I mean if we did, then it 
>> could internally know what are the modifier keys for the current 
>> platform and you wouldn't have that set of ORs.
>
> As far as I understand no we do not have such methods, we rarely use 
> keyCode for modifiers keys itself, usually we use the modifiers masks 
> which are applies to some other key events.
>
>> Why was the return value being ignored here ? I presume this method 
>> must have a side-effect that is part of what you are describing ? 
>> 7113 Component component = renderer.getTableCellRendererComponent(
>> 7114 JTable.this, null, false, false,
>
> This code was added back in 1997 as a first draft of accessibility 
> support for JTable.
> Initially we use this component in the similar way as an editor in the 
> current fix, something like this:
> ================
> Component component = renderer.getTableCellRendererComponent(
>                       JTable.this, null, false, row, column);
> if (component instanceof Accessible) {
>    return (Accessible) component;
> } else {
>   return new AccessibleJTableCell(column, row, JTable.this);
> }
> ================
>
> But since then we move this code to the AccessibleJTableCell itself:
>
> ================
> 8023         protected class AccessibleJTableCell extends 
> AccessibleContext
> 8024             implements Accessible, AccessibleComponent {
> ...........
> ...........
> 8068             protected AccessibleContext 
> getCurrentAccessibleContext() {
> 8069                 TableColumn aColumn = 
> getColumnModel().getColumn(column);
> 8070                 TableCellRenderer renderer = 
> aColumn.getCellRenderer();
> 8071                 if (renderer == null) {
> 8072                     Class<?> columnClass = getColumnClass(column);
> 8073                     renderer = getDefaultRenderer(columnClass);
> 8074                 }
> 8075                 Component component = 
> renderer.getTableCellRendererComponent(
> 8076                                   JTable.this, getValueAt(row, 
> column),
> 8077                                   false, false, row, column);
> 8078                 if (component instanceof Accessible) {
> 8079                     return component.getAccessibleContext();
> 8080                 } else {
> 8081                     return null;
> 8082                 }
> 8083             }
> ...........
> ...........
> 8092             protected Component getCurrentComponent() {
> 8093                 TableColumn aColumn = 
> getColumnModel().getColumn(column);
> 8094                 TableCellRenderer renderer = 
> aColumn.getCellRenderer();
> 8095                 if (renderer == null) {
> 8096                     Class<?> columnClass = getColumnClass(column);
> 8097                     renderer = getDefaultRenderer(columnClass);
> 8098                 }
> 8099                 return renderer.getTableCellRendererComponent(
> 8100                                   JTable.this, null, false, false,
> 8101                                   row, column);
> 8102             }
> ================
>
>> 7115 row, column); -phil
>>
>>
>>
>> On 7/10/19 10:16 PM, Sergey Bylokhov wrote:
>>> Hello.
>>> Please review the fix for JDK 13.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8226653
>>> Fix: http://cr.openjdk.java.net/~serb/8226653/webrev.01
>>>
>>> Short description:
>>>
>>>   If the user starts to edit the cell in the "default" JTable then we
>>> show the text editor in the cell but did not transfer the focus to it.
>>> This feature breaks the accessibility tools, which cannot report
>>> information about the current editor.
>>> I suggest to try it in the SwingSet2 to see how this feature actually
>>> works. The idea of the current fix is to disable this feature + some
>>> other related small fixes.
>>>
>>>
>>> Long description(from small changes to big):
>>>
>>>  - CAccessible.java:
>>>     When the user completes the editing of the cell we post
>>>     ACCESSIBLE_TABLE_MODEL_CHANGED event, and we need to notify the
>>>     native system that the data inside the table was changed.
>>>
>>>  - JTable.processKeyBinding():
>>>     1) We do not want to start editing of the cell if the only modifier
>>>        key is pressed, but some of the modifier keys were missed:
>>>        VK_ALT_GRAPH and KeyEvent.VK_META(which is CMD key on the 
>>> macOS).
>>>     2) To enable the "surrendersFocusOnKeystroke" property, I have 
>>> to add
>>>        one more flag, because I cannot do this via the public API:
>>> https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/javax/swing/JTable.html#setSurrendersFocusOnKeystroke(boolean) 
>>>
>>>        Note that it is specified as "false" by default, and setting it
>>>        to "true" even under the Accessibility tool may break the spec.
>>>        Probably we need to clarify this behavior in JDK 14.
>>>
>>>  - JTable.java.getAccessibleAt()/getAccessibleChild():
>>>     The accessibility API represent the table as a grid of cells, 
>>> and ignores
>>>     the real child of the table. Each cell is represented as a
>>>     AccessibleJTableCell, this AccessibleJTableCell is used as a 
>>> proxy object
>>>     and transfer all requests to the real component which is 
>>> responsible to
>>>     draw the cell. Note that it is specified that the render is 
>>> returned by
>>>     some of its methods:
>>> https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/javax/swing/JTable.AccessibleJTable.AccessibleJTableCell.html#getCurrentAccessibleContext() 
>>>
>>>     So we cannot modify these methods to return context of the 
>>> editor, instead
>>>     I added the switch before we return "AccessibleJTableCell".
>>>     In JDK 14 it can be moved to the "right" place.
>>>
>>>
>>> Addition thoughts about "FocusOnKeystroke" feature:
>>>  - We can leave this feature as-is and report the current editor to
>>>    the accessibility tool even if the editor is not focused, but 
>>> this may
>>>    confuse the user because it will not be possible to move the 
>>> cursor or
>>>    select the text from the keyboard and this is not expected 
>>> behavior of the
>>>    text editor.
>>>  - We also can disable the editing of the cell on the fly, and start 
>>> it only
>>>    if the user emulates the mouse click inside the table.
>>>  - Or we can leave everything as-is and move the responsibility for 
>>> this
>>>    feature to the application, which will enable or disable 
>>> "FocusOnKeystroke" feature.
>>>
>>
>
>