On Thu, 31 Jul 2025 15:05:20 GMT, Martin Fox <m...@openjdk.org> wrote:
>> This PR fixes numerous bugs in the handling of setMaximized() on macOS and >> also cleans up some issues seen when the user changes the maximized state >> manually. >> >> - After setMaximized(false) a notifyResize event was never sent so the width >> and height tracked by JavaFX was wrong. >> >> - During maximize and restore a series of notifyMove events were issued >> causing the maximized property to flip-flop during the animation. >> >> - Transparent windows were being maximized by passing native screen >> coordinates to a routine that expected JavaFX screen coordinates so the Y >> axis was flipped and the window was positioned incorrectly. >> >> - The restored frame is in native screen coordinates which was sent to a >> routine that expected flipped JavaFX coordinates. That routine corrected >> this by directly inspecting the call stack to see which routine was calling >> it. >> >> - When the user maximizes or restores the window a notifyResize event was >> always sent immediately stating that the window was maximized even when it >> was not. Then a series of notifyMove events were issued causing the >> maximized flag to flip-flop during the animation. >> >> This PR cleans all of this up. When using the setMaximized API only one >> notifyMove and one notifyResize event are sent and transparent windows are >> positioned correctly. When the user maximizes or restores manually we still >> see a series of notifyMove and notifyResize events but at least they all >> report the window’s state correctly. >> >> System tests are currently being written as part of #1789. It’s hard to >> create a system test that catches things like the mis-alignment of maximized >> transparent windows so that needs to be tested manually. The reproducer >> attached to the bug report can be used to verify this and also to see what >> happens when the user toggles the maximization state manually. >> >> Note that when the test programs tags something as “unexpected” it just >> means it saw a property change outside of any JavaFX API call. This is >> actually expected if the user manipulates the window manually. > > Martin Fox has updated the pull request incrementally with one additional > commit since the last revision: > > Cleaned up diff against master. Tighted up system test (tested on Windows > and Linux) I did some initial testing and ran a headful test through our CI system and it looks good. I left one comment on the test and will review the native glass code changes next. tests/system/src/test/java/test/javafx/stage/RestoreStagePositionTest.java line 130: > 128: > 129: Util.runAndWait(() -> stage.setMaximized(true)); > 130: Util.waitForIdle(stage.getScene()); I know that @andy-goryachev-oracle suggested its use, but for tests that do various windowing operations (maximize, full screen, restore, etc), `waitForIdle` is too short (its on the order of 160 msec). To make the test more robust, I recommend reverting this back to the `sleep(800)` you had earlier. Arguably, we should have a utility method for this sort of "wait for windowing changes to settle down", but we don't. ------------- PR Review: https://git.openjdk.org/jfx/pull/1860#pullrequestreview-3092704505 PR Review Comment: https://git.openjdk.org/jfx/pull/1860#discussion_r2257246639
