On Tue, 5 Nov 2024 06:12:20 GMT, Jayathirth D V <j...@openjdk.org> wrote:
> This PR removes AccessController.doPrivileged() calls in > javafx.graphics/com.sun.glass. It is part of umbrella task > [JDK-8342441](https://bugs.openjdk.org/browse/JDK-8342441). > > Also wherever classes are implementing PrivilegedAction they are replaced > with java.util.Supplier and get(). > > I have removed reference to all AccessControl** class except in > `Accessible.java` assuming that AccessControlContext from this class might be > needed at some other place and its better if we remove it under > [JDK-8342993](https://bugs.openjdk.org/browse/JDK-8342993) I see one problem in Accessibility. To reproduce, run any program that has controls, e.g., HelloTextArea, on either macOS or Windows and turn on the screen reader (VoiceOver on Mac or Narrator on Windows). You will get a continuous stream of the exceptions, for example on macOS: Exception in thread "JavaFX Application Thread" java.lang.ClassCastException: class com.sun.glass.ui.Accessible$GetAttribute cannot be cast to class javafx.scene.AccessibleRole (com.sun.glass.ui.Accessible$GetAttribute and javafx.scene.AccessibleRole are in module javafx.graphics@24-internal of loader 'app') at javafx.graphics@24-internal/com.sun.glass.ui.mac.MacAccessible.getNativeAccessible(MacAccessible.java:832) at javafx.graphics@24-internal/com.sun.glass.ui.View.getAccessible(View.java:1128) The rest looks good. I left a couple minor comment and a note for @johanvos asking if he wants to review the /ios/ file. modules/javafx.graphics/src/main/java/com/sun/glass/ui/Accessible.java line 169: > 167: getAttribute.attribute = attribute; > 168: getAttribute.parameters = parameters; > 169: return getAttribute; This needs to be `getAttribute.get()` to be equivalent to the previous code. modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkRobot.java line 47: > 45: boolean isOnWayland = ((Supplier<Boolean>) () -> { > 46: String waylandDisplay = System.getenv("WAYLAND_DISPLAY"); > 47: return waylandDisplay != null && !waylandDisplay.isBlank(); Minor: the Supplier seems unnecessary here, so this could be further simplified as: String waylandDisplay = System.getenv("WAYLAND_DISPLAY"); boolean isOnWayland = waylandDisplay != null && !waylandDisplay.isBlank(); modules/javafx.graphics/src/main/java/com/sun/glass/ui/ios/IosApplication.java line 40: > 38: private static native void _initIDs(); // init IDs for java callbacks > from native > 39: static { > 40: Application.loadNativeLibrary(); @johanvos This looks good to me, but since it is in ios code, do you want to check it as well? modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacApplication.java line 130: > 128: return !"false".equalsIgnoreCase(taskbarAppProp); > 129: }).get(); > 130: isTaskbarApplication = tmp; Minor: I think this can be further simplified by removing the `tmp` variable and the `Supplier` and assigning `isTaskbarApplication` directly on line 128. ------------- Changes requested by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1624#pullrequestreview-2415656981 PR Review Comment: https://git.openjdk.org/jfx/pull/1624#discussion_r1829443237 PR Review Comment: https://git.openjdk.org/jfx/pull/1624#discussion_r1829369148 PR Review Comment: https://git.openjdk.org/jfx/pull/1624#discussion_r1829373735 PR Review Comment: https://git.openjdk.org/jfx/pull/1624#discussion_r1829376484
