RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true

[Ambarish Rapte]

On Thu, 1 Dec 2022 17:03:03 GMT, Karthik P K <duke at openjdk.org> 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