On Mon, 7 Sep 2026 14:37:26 GMT, Marius Hanl <[email protected]> wrote:
>> Thiago Milczarek Sayao has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review feedback:
>> - Fix programmatically resizing unresizable windows
>> - Fix removing minimize on disabling the window (only works on older
>> window managers)
>
> modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp line
> 138:
>
>> 136: const char* categories = env->GetStringUTFChars(logCategories,
>> nullptr);
>> 137: glass_gtk_log_init(categories);
>> 138: env->ReleaseStringUTFChars(logCategories, categories);
>
> as `categories` could be null if I understand correctly, should the
> `ReleaseStringUTFChars` be called with `null`?
Added the null check
> modules/javafx.graphics/src/main/native-glass/gtk/GlassView.cpp line 272:
>
>> 270: }
>> 271:
>> 272:
>
> empty newline
Fixed
> modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 132:
>
>> 130:
>> 131: if (frame_type != TITLED) {
>> 132: initial_wmf = GDK_FUNC_ALL;
>
> Should `current_wmf` be initialized here as well?
Fixed
> tests/manual/stage/TestStage.java line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2025 Oracle and/or its affiliates. All rights reserved.
>
> Here and the other new files:
> Copyright should be 2026.
>
> and isn't the line: `Copyright (c) 2026, Oracle ...`, so with a comma?
Fixed
> tests/system/src/test/java/test/robot/javafx/stage/StageLocationTest.java
> line 53:
>
>> 51: import static test.util.Util.TIMEOUT;
>> 52:
>> 53: @Timeout(value = TIMEOUT, unit= TimeUnit.MILLISECONDS)
>
> minor: missing space right after unit
> Suggestion:
>
> @Timeout(value = TIMEOUT, unit = TimeUnit.MILLISECONDS)
Fixed
> tests/system/src/test/java/test/util/Util.java line 61:
>
>> 59: */
>> 60: public class Util {
>> 61: public static final String PARAMETERIZED_TEST_DISPLAY =
>> "{displayName} [{index}] {arguments}";
>
> Is this needed?
Yes, it's the format to show on the test report.
> tests/system/src/test/java/test/util/Util.java line 72:
>
>> 70: * to be processed. On Linux, these operations are asynchronous.
>> 71: * The native side may adjust values after Glass has applied them on
>> the Java side.
>> 72: * <p>
>
> Here and below, the empty `<p>` tag is not needed.
Removed the tag
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952508417
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952508852
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952501815
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952518576
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952509499
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952512646
PR Review Comment: https://git.openjdk.org/jfx/pull/2139#discussion_r3952519191
