On Mon, 27 May 2024 19:41:18 GMT, Marius Hanl <[email protected]> wrote:
>> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog`
>> is broken when `sizeToScene()` was called before or after.
>>
>> The approach here is to ignore the `sizeToScene()` request when the `Stage`
>> is maximized or set to fullscreen.
>> Otherwise the Window Manager of the OS will be confused and you will get
>> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize'
>> button still shows the 'Restore' Icon, while we already resized the `Stage`
>> to not be maximized).
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clarify in Javadoc
The fix looks good. I left one comment on the docs, but otherwise OK.
The test fails for me on Linux. Adding a sleep(500) before the assert checks
fixes it (there is no reliable way to ensure certain operations without a
sleep). I left inline comments. I also left one question regarding the per-test
cleanup method.
modules/javafx.graphics/src/main/java/javafx/stage/Window.java line 295:
> 293: * Set the width and height of this Window to match the size of the
> content
> 294: * of this Window's Scene.
> 295: * This request might be ignored if the Window is not allowed to do
> so, e.g. a {@link Stage}
Maybe add a `<p>` tag here?
Also, I recommend spelling out`for example,` (and adding a missing comma).
tests/system/src/test/java/test/javafx/stage/SizeToSceneTest.java line 64:
> 62: @AfterEach
> 63: void afterEach() {
> 64: Util.runAndWait(() -> mainStage.hide());
Do you need to check for `mainStage != null`?
tests/system/src/test/java/test/javafx/stage/SizeToSceneTest.java line 67:
> 65: }
> 66:
> 67: private static void assertStageScreenBounds() {
I recommend adding `Util.sleep(500);` here.
tests/system/src/test/java/test/javafx/stage/SizeToSceneTest.java line 75:
> 73: }
> 74:
> 75: private static void assertStageSceneBounds() {
I recommend adding `Util.sleep(500);` here.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1382#pullrequestreview-2100228766
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1628399428
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1628402170
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1628460133
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1628460259
