On Wed, 4 Jun 2025 14:48:31 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Implementation of >> [`StageStyle.EXTENDED`](https://gist.github.com/mstr2/0befc541ee7297b6db2865cc5e4dbd09). > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > enable preview feature system properties for tests Testing looks good. One thing I noticed on Linux when running on an older version of Ubuntu is that the buttons are always on the right, even when the window manager is configured to put them on the left for decorated windows. It might be worth considering a future enhancement to query the window manager for whether the buttons should be on the left or right. I spot checked a few things in the code and at least looked at all of the native code. I didn't spot any concerns. I did leave a couple questions, but nothing that would prevent my approving this. This is a nice piece of work, and I'm happy we will be getting it into JavaFX 25 as a preview feature! build.gradle line 2186: > 2184: systemProperty > 'junit.jupiter.execution.timeout.lifecycle.method.default', > JUNIT_LIFECYCLE_TIMEOUT > 2185: systemProperty 'javafx.enablePreview', 'true' > 2186: systemProperty 'javafx.suppressPreviewWarning', 'true' This seems a good approach. it might make it a little more difficult if we wanted to test the logic that throws an exception when `javafx.enablePreview` is not `"true"`, but that would still be possible by having that test explicitly set the property to the empty string in an `@Before` method, and then restoring it in an `@After` method. Probably not worth doing anyway. modules/javafx.graphics/src/main/java/com/sun/glass/ui/Window.java line 1403: > 1401: } > 1402: > 1403: private class UndecoratedMoveResizeHelper { Have you verified that removing this doesn't result in any loss of functionality? modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkWindow.java line 43: > 41: > 42: if (isExtendedWindow()) { > 43: > prefHeaderButtonHeightProperty().subscribe(this::onPrefHeaderButtonHeightChanged); This subscription is never canceled. Could this result in a leak? ------------- Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1605#pullrequestreview-2897115260 PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r2126830663 PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r2126871708 PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r2126869376
