On Sat, 28 Mar 2026 13:38:10 GMT, Copilot <[email protected]> wrote: >> Thiago Milczarek Sayao has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Location can be negative > > modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkCommonDialogs.java > line 52: > >> 50: String folder, >> 51: String title, >> 52: boolean usePortal); > > The native method parameter is named `usePortal`, but the value passed is > `DISABLE_PORTAL_FILE_CHOOSER` and the C++ side treats the flag as > `disablePortal`. Please rename the parameter (and/or constant) so the boolean > semantics are consistent (e.g., `disablePortal`) to avoid accidentally > inverting the logic in future changes.
Fixed > modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp line 696: > >> 694: >> 695: gdk_pixbuf_get_from_window (window, srcx, srcy, width, height); >> 696: return ret; > > `glass_pixbuf_from_window` declares `ret` but never assigns the result of > `gdk_pixbuf_get_from_window`, so it always returns NULL. Even if current call > sites were switched to use `gdk_pixbuf_get_from_window` directly, this > wrapper should either be fixed to return the pixbuf or removed to avoid > future regressions / accidental use. Fixed > tests/system/src/test/java/test/robot/javafx/stage/StageAttributesTest.java > line 195: > >> 193: assertTrue(topStage.isFullScreen()); >> 194: >> 195: // screen stage should take over the bottom stage > > Comment typo: "screen stage" looks like it should be "fullscreen stage" (to > match the test intent and surrounding assertions). > Suggestion: > > // fullscreen stage should take over the bottom stage Fixed > tests/system/src/test/java/test/util/Util.java line 361: > >> 359: } >> 360: }); >> 361: } > > `waitForBoolean` adds a listener but never removes it after the expected > value is observed. In longer test runs this can accumulate listeners on > long-lived properties (e.g., Stage properties). Consider removing the > listener once it fires (or using a weak listener) to avoid leaks and > unexpected extra notifications. Fixed ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004866590 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004866484 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004866976 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r3004866827
