On Mon, 3 Mar 2025 21:01:58 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 25 commits: >> >> - Merge remote-tracking branch 'origin/master' into >> 8349091.charts.thread.safety >> - review comments >> - Merge remote-tracking branch 'origin/master' into >> 8349091.charts.thread.safety >> - Merge remote-tracking branch 'origin/master' into >> 8349091.charts.thread.safety >> - enabled pie chart test >> - Merge branch 'master' into 8349091.charts.thread.safety >> - Merge branch 'master' into 8349091.charts.thread.safety >> - whitespace >> - Merge remote-tracking branch 'origin/master' into >> 8349091.charts.thread.safety >> - cleanup >> - ... and 15 more: https://git.openjdk.org/jfx/compare/7a7854c9...4288d1d0 > > modules/javafx.controls/src/main/java/javafx/scene/chart/AreaChart.java line > 546: > >> 544: symbol.setAccessibleRole(AccessibleRole.TEXT); >> 545: symbol.setAccessibleRoleDescription("Point"); >> 546: symbol.setFocusTraversable(isAccessibilityActive()); > > So, if I understand correctly, we can't directly do this `bind`: > > > symbol.focusTraversableProperty().bind(Platform.accessibilityActiveProperty()) > > Because on a background thread there may not yet be an initialized > `Platform`? Or perhaps, `Platform` would need to initialize and we don't > want to do that yet on a background thread? The reason we `should not` be binding in this PR is because in part it's **bad design(tm)**: we are creating a zillion of listeners (one per plot data point); a better solution would be to create one listener and use (already existing series) to toggle the data points. The other reason is https://bugs.openjdk.org/browse/JDK-8351067 - the `Platform::accessibilityActive` property getter is not thread safe, but even if was, registering a listener with it may cause the change to come in **before** the node becomes a part of the scene graph, leading to unsynchronized multi-threaded access. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1978198906
