RFR: 8309374: Accessibility Focus Rectangle on ListItem is not drawn when ListView is shown for first time

[Kevin Rushforth]

On Tue, 13 Feb 2024 12:29:49 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

> This is accessibility specific fix.
> 
> **Issue**: When a ListView is shown for first time then accessibility focus rectangle is not drawn around the focused ListIem
> 
> **Cause:** 
>    The ListView takes a little time to create it's skin(ListViewSkin) and the skins for ListItems(ListCellSkin)
>    If the Accessibility client application requests focused item before ListView is completely ready then JavaFX return null.
> 
> **Fix**: Send a focus item change notification to accessibility client application after the ListIteam is ready
> 
> **Verification:**
> - On Windows machine, launch Narrator. Issue is observed only with Narrator
> - Execute the [program](https://bugs.openjdk.org/secure/attachment/104180/Main.java) attached to the JBS issue [JDK-8309374](https://bugs.openjdk.org/browse/JDK-8309374)
>      Move through the TextFields and press `Alt+down`, observe that focus rectangle is drawn correctly.
>      Once the ListView is showing Press and hold `Up/Down` or `Ctrl + Up/Down` keys, observe that focus rectangle is always drawn.

The fix looks good to me, and my initial testing shows that it works as expected. I left a few suggestions on minor wording changes inline. I'll reapprove if you make the changes.

modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line 351:

> 349: 
> 350:     /*
> 351:      * The layoutChildren() method is overloaded to address a specific accessibility issue: JBS-8309374

Minor typos: "overloaded" --> "overridden"; "JBS" --> "JDK"

modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line 366:

> 364:             if (listView != null) {
> 365:                 /*
> 366:                  * The notifyAccessibleAttributeChanged() call is submitted on the Application thread, because:

This method is already being run on the application thread.

Suggestion: "is submitted via runLater to defer it until after the layout completes, because:"

modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line 371:

> 369:                  * notification from here.
> 370:                  * We observed that this scenario occurs when client application is trying to get the
> 371:                  * UIA_HasKeyboardFocusPropertyId property of a focused ListItem's parent(ListView).

I might replace `UIA_HasKeyboardFocusPropertyId`, which is Windows-specific, with something more generic like "the focus item".

modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line 372:

> 370:                  * We observed that this scenario occurs when client application is trying to get the
> 371:                  * UIA_HasKeyboardFocusPropertyId property of a focused ListItem's parent(ListView).
> 372:                  * This scenario is avoided by submitting the call notifyAccessibleAttributeChanged() on Application thread,

Similar comment to the earlier one about the thread. I suggest something like:

"This scenario is avoided by submitting the call via runLater,"

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1363#pullrequestreview-1878722288
PR Review Comment: https://git.openjdk.org/jfx/pull/1363#discussion_r1488437475
PR Review Comment: https://git.openjdk.org/jfx/pull/1363#discussion_r1488442179
PR Review Comment: https://git.openjdk.org/jfx/pull/1363#discussion_r1488445070
PR Review Comment: https://git.openjdk.org/jfx/pull/1363#discussion_r1488447524