8271846 a11y API lacks setSelectedIndex method #8578
Conversation
|
👋 Welcome back asemenov! A progress list of the required criteria for merging this PR into |
openjdk
bot
added
client
Webrevs
|
| /** | ||
| * This interface provides list specific data. | ||
| * | ||
| * @author Artem Semenov |
| @@ -0,0 +1,223 @@ | |||
| /* | |||
| * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | |||
| * Copyright (c) 2021, JetBrains s.r.o.. All rights reserved. | |||
|
/csr |
|
/reviewers 2 |
|
@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request. |
|
Since you are propsosing new API, you can't backport this. I hope that is understood. |
|
@prrace |
|
/addissue 8286674 |
|
@savoptik Unknown command |
|
/add issue 8286674 |
|
@savoptik Unknown command |
|
/help |
|
@savoptik Available commands:
|
|
/issue add 8286674 |
|
@savoptik |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
Just some comment so that the pull request doesn't close. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
Comment to prevent the pull request from closing. |
|
@azuev-java @prrace please review this CSR ticket JDK-8286674 |
Before we get to that
|
|
/issue help |
|
@savoptik Command syntax:
Some examples:
If issues are specified only by their ID, the title will be automatically retrieved from JBS. The project prefix ( |
|
/issue remove 8286674 |
|
@prrace Thanks a lot. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@azuev-java @mrserb @prrace please review CSR and this PR. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
No, you don't need to close this pull request. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
No, you don't need to close this pull request. |
|
I am still waiting for the following questions (from July) to be answered here (not somewhere else)
By which I mean, if this needs to be solved in say JDK 11 too, what will you do ??
Also leaving aside that you haven't yet shown the API is needed I note that there are javax.accessibility is NOT The similar examples are nested classes of the component, eg In fact there's even already a JList.AccessibleJList ! And it seems to have things that look similar to things you say you need and your API
=== and ===
etc etc .. |
|
@prrace The absence of the dedicated AccessibleList interface does not allow setting selections on lists on MacOS using the accessible shortcuts and external accessibility devices. The current solution severely limits the ability of accessibility subsystem to interact with the list component thus it is not acceptable. Existing implementation uses AccessibleSelection implemented in the JList but it lacks ability to work with multiple selection intervals and side effects of setting singular selection with sequential calling of clearSelection() and addSelection() lead to the the line selection action being repeated by the VoiceOver indefinitely. We need a new accessibility interface that will implement the selection model for accessibility lists. Also it should provide additional information about the list, regardless of whether the accessibility list is inherited from JList or impolemented from scratch. New interface should be able to provide system with the additional information such as selection mode and should be flexible enough to allow implementation of both single and multiselection modes. The AccessibleSelection interface does not allow working with index intervals, which will prevent the implementation of multiple selection in the future. |
| import javax.accessibility.AccessibleStateSet; | ||
| import javax.accessibility.AccessibleText; | ||
| import javax.accessibility.AccessibleValue; | ||
| import java.util.*; |
There was a problem hiding this comment.
We are trying not to use wildcard imports - probably automatic optimization of the IDE did it? I am always turning it off or set margin so high it is not triggered on anything with less then 200 individual package imports.
| * @test | ||
| * @bug 8271846 | ||
| * @summary Test implementation of AccessibleList interface | ||
| * @author Artem.Semenov@jetbrains.com |
There was a problem hiding this comment.
We are not using author tags in tests either - but that's just a nitpick.
You didn't speak to the duplicated functionality. |
Not 2022 ?
…zation of the IDE did it? I am always turning it off or set margin so high it is not triggered on anything with less then 200 individual package imports.
savoptik
force-pushed
the
asemenov/JDK-8271846
branch
from
4a9bc8e to
ff82513
Compare
|
@savoptik Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See OpenJDK Developers’ Guide for more information. |
| ((AccessibleList) pat).setSelectionInterval(i, i); | ||
| return; | ||
| } | ||
| as.addAccessibleSelection(i); |
There was a problem hiding this comment.
I would like to clarify one initial question. Why we cannot use clearAccessibleSelection to clean the current selection and then add a new one by the addAccessibleSelection?
There was a problem hiding this comment.
We tried to do this, it leads to an endless repetition of the selected line until it freezes.
There was a problem hiding this comment.
Probably because of the endless notifications loop? implementation of addSelectionInterval looks similar to setSelectionInterval. In some cases the addSelectionInterval just call setSelectionInterval.
There was a problem hiding this comment.
It looks like it is. Please formulate the question differently.
There was a problem hiding this comment.
It is unclear why the setSelectionInterval is actually any better than addAccessibleSelection. Both have a similar implementation, in some cases, the addSelectionInterval just calls setSelectionInterval, and seems that posts similar notifications. So it is unclear why the setSelectionInterval works fine and addAccessibleSelection does not work.
There was a problem hiding this comment.
I tried calling clearSelection again instead of adding an interface:
if (as == null) return;
if (pac.getAccessibleRole().equals(AccessibleRole.LIST)) {
as.clearAccessibleSelection();
}
as.addAccessibleSelection(i);After that I changed the call to javax.swing.JList.AccessibleJList#addAccessibleSelection
JList.this.addSelectionInterval(i, i);on the
JList.this.getSelectionModel().setSelectionInterval(i, i);Nothing changed. VoiceOVer still freezes.
I think that the problem is not that setting the selection doesn't work, but that clearing the selection before setting causes the VO to fris.
There was a problem hiding this comment.
Some probably related points, the implementation of the addAccessibleSelection for the JComboBox. is the same:
clearAccessibleSelection();
JComboBox.this.setSelectedIndex(i);
It is possible that it also hangs?
There was a problem hiding this comment.
I think that the problem is not that setting the selection doesn't work, but that clearing the selection before setting causes the VO to fris.
Are you sure that the selection should be always cleared in this method, how it will work if the jlist supports multiline selection? Or voice over does not support multyline selection?
As a fix, you can delete listeners from the jlist so your request to clear the current selection will not be sent back to the voiceover. Or you can add a new method to the AccessibleSelection interface with the default implementation "clearAccessibleSelection+addAccessibleSelection". Then override it in the jlist to call setSelectedIndex().
|
@mrserb It is possible that it also hangs? I'm not sure if this should affect comboboxes. The combo box is arranged differently. When the ComboBox is opened, the focus is moved not to the list, but to the root panel, and from it it is proxied to the list items, which are a set of buttons that are not hierarchically or in any way connected. Are you sure that the selection should be always cleared in this method, how it will work if the jlist supports multiline selection? Or voice over does not support multyline selection? As stated in the pull request description. We cannot yet implement multiple selection with VO quick navigation, as this part of the A11y API is not described by Apple. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
Please do not close this pull request. |
|
@savoptik This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
Some comment so that the pull request is not closed. |
|
We will think about solving this problem and try to offer the best. |
4 participants
A11Y implementation on macOS has to directly call the 'JList.setSelectedIndex' method in order to request selection on an item (see 'CAccessibility.requestSelection'). The reason is that a11y API lacks appropriate method.There's only 'javax.accessibility.AccessibleSelection#addAccessibleSelection' which is mapped to 'javax.swing.JList#addSelectionInterval', it can not be used to set selected index.
@forantar @azuev-java @mrserb please review.
Please note that the new API allows you to implement a multiple selection in lists from the Java side, but I did not succeed in implementing it, because I could not determine the inclusion of the so-called "VoiceOver multiple selection mode".
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/8578/head:pull/8578$ git checkout pull/8578Update a local copy of the PR:
$ git checkout pull/8578$ git pull https://git.openjdk.org/jdk pull/8578/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 8578View PR using the GUI difftool:
$ git pr show -t 8578Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/8578.diff