On Tue, 13 Feb 2024 12:29:49 GMT, Ambarish Rapte <ara...@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
