On Thu, 4 Jul 2024 09:32:39 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> This PR fixes a long standing issue where the `Tooltip` will always wait one
>> second until it appears the very first time, even if the
>> `-fx-show-delay` was set to another value.
>>
>> The culprit is, that the `cssForced` flag is not inside `Tooltip`, but
>> inside the `TooltipBehaviour`. So the very first `Tooltip` gets processed
>> correctly, but after no `Tooltip` will be processed by CSS before showing,
>> resulting in the set `-fx-show-delay` to not be applied immediately.
>>
>> Added a bunch of headful and headless tests for the behaviour since there
>> were none before.
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add many more unit tests for Tooltip
I left a few inline comments, including a possible suggestion to fix the
intermittent test failures.
tests/system/src/test/java/test/robot/javafx/scene/TooltipTest.java line 64:
> 62:
> 63: private static void assertTooltipShowDelay(long tooltipShowTime, long
> expectedTime) {
> 64: assertTooltipShowDelay(tooltipShowTime, expectedTime, 50);
If I change this to 100, then all tests pass most of the time. So maybe 150 or
200 would be a better maximum delta?
If you do make this change, then you will need to find all of the three-arg
calls with a maxDifference less than that and change them to the two-arg
version.
tests/system/src/test/java/test/robot/javafx/scene/TooltipTest.java line 109:
> 107: long maximumTime = expectedTime + 100;
> 108:
> 109: assertTooltipShowDelay(tooltipShowTime, expectedTime,
> maximumTime);
The third argument is a maximum difference, not a max time. So unless there is
some reason to add the expected time twice, I would just use a larger constant
delta.. You might even be able to get rid of the third arg depending on what we
eventually go with for the default max delta.
tests/system/src/test/java/test/robot/javafx/scene/TooltipTest.java line 122:
> 120: assertTrue(tooltip.isShowing());
> 121:
> 122: assertTooltipShowDelay(tooltipShowTime, 30, 100);
This is an example of the three-arg call where you can eliminate the third arg
if you make the default > 100.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1394#pullrequestreview-2164415777
PR Review Comment: https://git.openjdk.org/jfx/pull/1394#discussion_r1669372751
PR Review Comment: https://git.openjdk.org/jfx/pull/1394#discussion_r1669376935
PR Review Comment: https://git.openjdk.org/jfx/pull/1394#discussion_r1669378097
