On Sat, 9 Mar 2024 18:41:10 GMT, Marius Hanl <mh...@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:
>
> improve tests
The fix looks good. The spec changes (updated javadocs) look good. Can you
create the CSR for the spec change?
I have a couple overall comments:
* I wanted to verify different orders of operation, so I wrote a (manual) test
program and attached it to the JBS bug. It covers the following cases:
* set ; sizeToScene ; show
* sizeToScene ; set ; show
* show ; set ; sizeToScene
* show ; sizeToScene ; set
I verified that the first 3 are broken today. All cases work with your fix. I
think it might be a good idea to add automated tests for the different
orderings.
* Please merge the latest master. Note that the calls to Util.shutdown in the
tests must be fixed after this is done or they will no longer compile.
tests/system/src/test/java/test/javafx/stage/SizeToSceneFullscreenTest.java
line 69:
> 67: @AfterAll
> 68: public static void teardown() {
> 69: Util.shutdown(stage);
You will need to remove the `stage` argument after you merge the latest master.
tests/system/src/test/java/test/javafx/stage/SizeToSceneMaximizeTest.java line
69:
> 67: @AfterAll
> 68: public static void teardown() {
> 69: Util.shutdown(stage);
You will need to remove the `stage` argument after you merge the latest master.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1382#pullrequestreview-2032228167
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1585330069
PR Review Comment: https://git.openjdk.org/jfx/pull/1382#discussion_r1585330320
