8284544: [Win] Name-Property of Spinner cannot be changed #1291

arapte · 2023-11-20T05:37:38Z

Currently we use the value of spinner as it's UIA_NamePropertyId when a11y client application requests for it.
Ideally we should use the text set by Node.setAccessibleText() as the UIA_NamePropertyId.
For other controls such as Slider, ListView we use the text set by setAccessibleText() API.

Fix:
Use the text set by Node.setAccessibleText() as the UIA_NamePropertyId.
This means, when a11y client requests UIA_NamePropertyId, which is mapped to AccessibleAttribute.TEXT attribute, we shall return the accessible text.
So we need another way to read out the VALUE of the Spinner.

For this we need to implement IValueProvider pattern for Spinner control
IValueProvider requests the value of the control using it's API get_ValueString()
It required to introduce a new AccessibleAttribute VALUE_STRING
IValueProvider also reads out if the control is editable or not, hence added EDITABLE case in Spinner.queryAccessibleAttribute()

Verification:

Run any spinner app, with setAccessibleText set on spinner
Run Windows narrator and observe
Without this fix:
1. Narrator does not read the text set by setAccessibleText
1. In application "Accessibility Insights for Windows", you can see the value of Spinner as the Name property
After this fix:
1. Narrator reads the accessible text then value of Spinner and then if editable or not
1. In application "Accessibility Insights for Windows", you can see the text set by setAccessibleText() for Spinner as the Name property and the Spinner's value as value property

Progress

Change must not contain extraneous whitespace
Change requires CSR request JDK-8320505 to be approved
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

JDK-8284544: [Win] Name-Property of Spinner cannot be changed (Bug - P3)
JDK-8320505: [Win] Name-Property of Spinner cannot be changed (CSR)

Reviewers

Kevin Rushforth (@kevinrushforth - Reviewer)
Ajit Ghaisas (@aghaisas - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1291/head:pull/1291
$ git checkout pull/1291

Update a local copy of the PR:
$ git checkout pull/1291
$ git pull https://git.openjdk.org/jfx.git pull/1291/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1291

View PR using the GUI difftool:
$ git pr show -t 1291

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1291.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-11-20T05:38:25Z

👋 Welcome back arapte! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2023-11-20T05:42:46Z

Webrevs

kevinrushforth · 2023-11-20T13:18:28Z

/reviewers 2
/csr

openjdk · 2023-11-20T13:19:39Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

openjdk · 2023-11-20T13:20:14Z

@kevinrushforth has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@arapte please create a CSR request for issue JDK-8284544 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

kevinrushforth · 2023-11-20T14:59:47Z

This should be tested in connection with PR #1226

kevinrushforth

I left quick comment and a question inline.

modules/javafx.controls/src/main/java/javafx/scene/control/Spinner.java

modules/javafx.graphics/src/main/java/javafx/scene/AccessibleAttribute.java

arapte · 2023-11-22T17:17:41Z

Updated the PR with changes required for macOS, such that the Spinner is read same as before.

kevinrushforth

The changes look good and everything works as I would expect. I left one question, but it is fine as you have it.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacAccessible.java

aghaisas

The fix looks good!

openjdk · 2023-12-05T10:03:03Z

@arapte This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8284544: [Win] Name-Property of Spinner cannot be changed

Reviewed-by: kcr, aghaisas

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 32 new commits pushed to the master branch:

aedc887: 8320359: ImageView: add styleable fitWidth, fitHeight, preserveRatio, smooth properties
092d5d2: 8314597: Deprecate for removal protected access methods in converters
036d81b: 8269921: TextFlow: listeners on bounds can throw NPE while computing text bounds
0d33417: 8320267: WebView crashes on macOS 11 with WebKit 616.1
d65d8a6: 8316518: javafx.print.Paper getWidth / getHeight rounds values, causing errors.
c46c172: 8320773: [macOS] All IME input blocked
b80ec39: 8313648: JavaFX application continues to show a black screen after graphic card driver crash
3548cdd: 8320444: Column drag header is positioned wrong for nested columns
606878a: 8318387: Update GStreamer to 1.22.6
a1036b2: 8303826: Add FX test for JDK-8252255
... and 22 more: https://git.openjdk.org/jfx/compare/2aa69e0120976c77df35adcdb4908df1b55f8281...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

arapte · 2023-12-05T10:38:36Z

/integrate

openjdk · 2023-12-05T10:40:35Z

Going to push as commit 43448dd.
Since your change was applied there have been 33 commits pushed to the master branch:

09922d5: 8318388: Update libxslt to 1.1.39
aedc887: 8320359: ImageView: add styleable fitWidth, fitHeight, preserveRatio, smooth properties
092d5d2: 8314597: Deprecate for removal protected access methods in converters
036d81b: 8269921: TextFlow: listeners on bounds can throw NPE while computing text bounds
0d33417: 8320267: WebView crashes on macOS 11 with WebKit 616.1
d65d8a6: 8316518: javafx.print.Paper getWidth / getHeight rounds values, causing errors.
c46c172: 8320773: [macOS] All IME input blocked
b80ec39: 8313648: JavaFX application continues to show a black screen after graphic card driver crash
3548cdd: 8320444: Column drag header is positioned wrong for nested columns
606878a: 8318387: Update GStreamer to 1.22.6
... and 23 more: https://git.openjdk.org/jfx/compare/2aa69e0120976c77df35adcdb4908df1b55f8281...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-12-05T10:40:44Z

@arapte Pushed as commit 43448dd.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Add attribute VALUE_STRING for Spinner

39243a6

openjdk bot added the rfr Ready for review label Nov 20, 2023

openjdk bot added the csr Need approved CSR to integrate pull request label Nov 20, 2023

kevinrushforth mentioned this pull request Nov 20, 2023

8313556: Create implementation of NSAccessibilitySlider protocol #1226

Closed

3 tasks

kevinrushforth reviewed Nov 21, 2023

View reviewed changes

modules/javafx.controls/src/main/java/javafx/scene/control/Spinner.java Show resolved Hide resolved

modules/javafx.graphics/src/main/java/javafx/scene/AccessibleAttribute.java Show resolved Hide resolved

arapte added 2 commits November 22, 2023 09:16

add @SInCE 22

58bf6dd

Mac a11y change for VALUE_STRING attribute for Spinner

31f8d14

kevinrushforth self-requested a review November 28, 2023 22:16

kevinrushforth approved these changes Dec 2, 2023

View reviewed changes

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacAccessible.java Show resolved Hide resolved

openjdk bot removed the csr Need approved CSR to integrate pull request label Dec 4, 2023

aghaisas approved these changes Dec 5, 2023

View reviewed changes

openjdk bot added the ready Ready to be integrated label Dec 5, 2023

openjdk bot added the integrated Pull request has been integrated label Dec 5, 2023

openjdk bot closed this Dec 5, 2023

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8284544: [Win] Name-Property of Spinner cannot be changed #1291

8284544: [Win] Name-Property of Spinner cannot be changed #1291

arapte commented Nov 20, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Nov 20, 2023

mlbridge bot commented Nov 20, 2023 •

edited

Loading

kevinrushforth commented Nov 20, 2023

openjdk bot commented Nov 20, 2023

openjdk bot commented Nov 20, 2023

kevinrushforth commented Nov 20, 2023

kevinrushforth left a comment

arapte commented Nov 22, 2023

kevinrushforth left a comment

aghaisas left a comment

openjdk bot commented Dec 5, 2023

arapte commented Dec 5, 2023

openjdk bot commented Dec 5, 2023

openjdk bot commented Dec 5, 2023

8284544: [Win] Name-Property of Spinner cannot be changed #1291

8284544: [Win] Name-Property of Spinner cannot be changed #1291

Conversation

arapte commented Nov 20, 2023 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Nov 20, 2023

mlbridge bot commented Nov 20, 2023 • edited Loading

Webrevs

kevinrushforth commented Nov 20, 2023

openjdk bot commented Nov 20, 2023

openjdk bot commented Nov 20, 2023

kevinrushforth commented Nov 20, 2023

kevinrushforth left a comment

Choose a reason for hiding this comment

arapte commented Nov 22, 2023

kevinrushforth left a comment

Choose a reason for hiding this comment

aghaisas left a comment

Choose a reason for hiding this comment

openjdk bot commented Dec 5, 2023

arapte commented Dec 5, 2023

openjdk bot commented Dec 5, 2023

openjdk bot commented Dec 5, 2023

arapte commented Nov 20, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 20, 2023 •

edited

Loading