RFR: 8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v7]

[Kevin Rushforth]

On Mon, 27 May 2024 19:41:18 GMT, Marius Hanl <mhanl at openjdk.org> 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