On Sat, 17 Jul 2021 00:15:48 GMT, Jose Pereda <jper...@openjdk.org> wrote:
> As described in the issue, when the stage is moved to the screen top > position, it is moved below the system menu bar. However, doing it twice > doesn't trigger a native callback to the Java layer, and the stage yPosition > doesn't get updated with the actual position of the application. This has > several side effects, like the wrong popup control position. > > This PR adds a callback in case set position doesn’t match actual position. > > It is only going to be called when the final position of the stage doesn't > match the one that was set, which could happen in rare occasions, mainly due > to constrains applied by the native layer, so it doesn't add any overhead. > > A system test for MacOS is included. The fix and the test looks good. I left one suggestion and two minor formatting comments on the test. tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line 25: > 23: * questions. > 24: */ > 25: package test.javafx.stage; Minor: can you add a blank line before the `package` statement? (I realize we have a few existing classes that don't follow this) tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line 72: > 70: stage.setY(400); > 71: stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e -> > 72: > Platform.runLater(startupLatch::countDown)); Minor: indentation is a bit off if you intended to line it up. tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line 86: > 84: } > 85: } catch (InterruptedException ex) { > 86: fail("Unexpected exception: " + ex); If you add `throws Exception` to the method, you can simplify this and replace the try/catch block with: assertTrue("Timeout waiting for FX runtime to start", startupLatch.await(15, TimeUnit.SECONDS)); (most of our newer tests do this) ------------- PR: https://git.openjdk.java.net/jfx/pull/580