On Thu, 1 Dec 2022 17:03:03 GMT, Karthik P K <[email protected]> wrote:
> Cause: When slider is dragged for first time after tooltip appears,
> setOnMousePressed event was not invoked, hence dragStart was null in the
> subsequently invoked event handler (setOnMouseDragged).
>
> Fix: Initialized dragStart in initialize method.
>
> Test: Added system test to validate the fix.
>
> Note: Initializing dragStart in layoutChildren method as suggested in the bug
> was causing side effects. Hence initialized dragStart in initialize method.
Providing few quick comments about the test.
tests/system/src/test/java/test/robot/javafx/scene/SliderTooltipNPETest.java
line 78:
> 76: }
> 77: }
> 78:
`main()` method is not required, can be removed.
tests/system/src/test/java/test/robot/javafx/scene/SliderTooltipNPETest.java
line 84:
> 82: }
> 83:
> 84: private void dragSliderAfterTooltipDisplayed(int dragDistance,
> boolean xIncr) throws Throwable {
`boolean xIncr` is not required for the test. It can be removed along with it's
use on line 101.
tests/system/src/test/java/test/robot/javafx/scene/SliderTooltipNPETest.java
line 89:
> 87: Util.runAndWait(() -> {
> 88: robot.mouseMove((int)(scene.getWindow().getX() + scene.getX()
> + SCENE_WIDTH/2),
> 89: (int)(scene.getWindow().getY() + scene.getY() +
> SCENE_HEIGHT/2));
The x and y position calculation should consider the sliders layout as well.
I think it should be as:
robot.mouseMove((int)(scene.getWindow().getX() + scene.getX() +
slider.getLayoutX() + slider.getLayoutBounds().getWidth()/2),
(int)(scene.getWindow().getY() + scene.getY() +
slider.getLayoutY() + slider.getLayoutBounds().getHeight()/2));
and similar change on line 102.
Line 105 would get removed when removing `xIncr`
tests/system/src/test/java/test/robot/javafx/scene/SliderTooltipNPETest.java
line 92:
> 90: });
> 91:
> 92: Thread.sleep(3000); // Wait for tooltip to be displayed
This `sleep` can be removed, The sleep of 1000 ms on line 85 should be
sufficient.
I ran by removing this sleep on Mac, Test ran well.
Please check on other platforms.
-------------
Changes requested by arapte (Reviewer).
PR: https://git.openjdk.org/jfx/pull/965
