8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed #1394

Maran23 · 2024-03-08T15:58:09Z

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 tests for the behaviour since there were none before.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1394/head:pull/1394
$ git checkout pull/1394

Update a local copy of the PR:
$ git checkout pull/1394
$ git pull https://git.openjdk.org/jfx.git pull/1394/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1394

View PR using the GUI difftool:
$ git pr show -t 1394

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1394.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-03-08T15:58:39Z

👋 Welcome back mhanl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2024-03-08T16:02:08Z

Webrevs

openjdk · 2024-03-08T16:03:46Z

@Maran23 Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

openjdk · 2024-03-08T16:05:21Z

@Maran23 Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

…st tooltip that is shown before it is displayed

openjdk · 2024-03-08T16:07:15Z

@Maran23 Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

andy-goryachev-oracle · 2024-03-08T22:36:19Z

Testing the combined (this PR merged with the latest master) on macOS 14.3.1 M1 Pro (though this is probably not important), using the latest MonkeyTester
https://github.com/andy-goryachev-oracle/MonkeyTest

select Tooltip page
open the Tools -> CSS Playground tool
type .tooltip { -fx-show-delay:1ms; } into Custom CSS text area, press Update button. (this updates the stylesheet in all the open windows)
hover over the tooltip page

What I see is the very first time the tooltip appears it's using the old show-delay value of 1000ms. Any subsequent attempts the new delay is in effect.

Then, change back to 1000ms. Same thing happens: the first time the old timeout value is used instead of the new.

andy-goryachev-oracle · 2024-03-08T22:41:50Z

There is also another problem (though it is a separate issue as it can be seen with the master branch):

set the -fx-show-delay to 1ms as described in the previous comment
set the Graphic: to "Image"

Notice how sometimes, when slowly entering the tooltip page from the left side, the tooltip appears and then disappears. Async code is hard!

edit 2024/04/12: created https://bugs.openjdk.org/browse/JDK-8330187

andy-goryachev-oracle · 2024-03-08T22:47:41Z

the disappearing tooltip clip (will move to its own ticket later):

video1956899183.mp4

andy-goryachev-oracle · 2024-03-08T22:59:42Z

This is another, unrelated issue: If I specify a number instead of duration like -fx-show-delay: 1; we'll get an exception every time the tooltip is about to be shown.

Exception in thread "JavaFX Application Thread" java.lang.ClassCastException: class java.lang.Double cannot be cast to class javafx.util.Duration (java.lang.Double is in module java.base of loader 'bootstrap'; javafx.util.Duration is in module javafx.base of loader 'app')
	at javafx.controls/javafx.scene.control.Tooltip.getShowDelay(Tooltip.java:334)
	at javafx.controls/javafx.scene.control.Tooltip$TooltipBehavior.lambda$0(Tooltip.java:1015)
	at javafx.base/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247)
	at javafx.base/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.base/javafx.event.Event.fireEvent(Event.java:199)
	at javafx.graphics/javafx.scene.Scene$MouseHandler.process(Scene.java:3987)
	at javafx.graphics/javafx.scene.Scene.processMouseEvent(Scene.java:1893)
	at javafx.graphics/javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2711)
	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:411)
	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:1)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$2(GlassViewEventHandler.java:450)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:430)
	at javafx.graphics/com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:449)
	at javafx.graphics/com.sun.glass.ui.View.handleMouseEvent(View.java:551)
	at javafx.graphics/com.sun.glass.ui.View.notifyMouse(View.java:937)
	at javafx.graphics/com.sun.glass.ui.mac.MacView.notifyMouse(MacView.java:128)

Once that happens, the exception will get thrown every time, even after the correct CSS is entered. The app must be restarted.

But we don't get an exception for a boolean value: -fx-wrap-text:"yo"; or a double value like .tooltip { -fx-graphic-text-gap:"true"; }. The latter will correctly issue a warning

WARNING: Failed to set css [-fx-graphic-text-gap] on [DoubleProperty [bean: javafx.scene.control.Tooltip@468235b3, name: graphicTextGap, value: 4.0]] due to 'class java.lang.Boolean cannot be cast to class java.lang.Number (java.lang.Boolean and java.lang.Number are in module java.base of loader 'bootstrap')'

edit 2024/11/12: created https://bugs.openjdk.org/browse/JDK-8330186

hjohn

Although the change is an improvement, I think a similar problem will still be present when CSS changes. The cssForced flag I think needs to reset to false whenever a CSS styleable property of the tool tip is modified. That would include all such properties...

This is a bit of a general issue in JavaFX (it really shouldn't be possible for any Node to be shown that hasn't had CSS processed for it), yet this can easily happen, and depending on how different the default style is versus the intended style, this can lead to a visual artifact (most noticable is for example a white background, when it is supposed to be black or transparent).

Perhaps we should open an issue to solve this for all controls in one go? A solution that would prevent any Node from being displayed without CSS applied?

Maran23 · 2024-03-09T09:10:28Z

What I see is the very first time the tooltip appears it's using the old show-delay value of 1000ms. Any subsequent attempts the new delay is in effect.

Then, change back to 1000ms. Same thing happens: the first time the old timeout value is used instead of the new.

There seems to be a difference when using setStyle and a global stylesheet. I'm investigating.

Maran23 · 2024-03-09T09:13:55Z

Perhaps we should open an issue to solve this for all controls in one go? A solution that would prevent any Node from being displayed without CSS applied?

Yes, that sounds like a good idea. Checking the calls to applyCss, it is mostly in table related classes and I remember that I already asked myself the question whether they are really needed there.

Although the change is an improvement, I think a similar problem will still be present when CSS changes. The cssForced flag I think needs to reset to false whenever a CSS styleable property of the tool tip is modified. That would include all such properties...

Will check. Tooltip is unfortunately a corner case where we to get some CSS properties BEFORE we show it in order to correctly setup the animation timeline.

…tylesheets are considered for the e.g. tooltip show-delay

modules/javafx.controls/src/main/java/javafx/scene/control/Tooltip.java

Maran23 · 2024-03-09T12:20:12Z

This is another, unrelated issue: If I specify a number instead of duration like -fx-show-delay: 1; we'll get an exception every time the tooltip is about to be shown.

This is because the CSS implementation thinks it is a PX value and therefore parse it wrong. You can check the CssParser to see why this happens. Fix might be simple, but the code in the parser is very error prone for this kind of stuff.

(And Note that the StyleableProperty does not do any typecheck, therefore a double value can be set where a Duration would be expected, which is very very bad)

openjdk · 2024-03-13T19:59:52Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

bridgekeeper · 2024-04-10T21:09:35Z

@Maran23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Maran23 · 2024-04-10T21:11:14Z

keep it open.

andy-goryachev-oracle · 2024-04-11T20:02:37Z

Please sync up with the latest master (should be a SOP for any long outstanding PRs - sorry for long delay!)

Description	Resource	Type	Path	Location
The method shutdown() in the type Util is not applicable for the arguments (Stage)	TooltipTest.java	Java Problem	/systemTests-test/java/test/robot/javafx/scene	line 238

^ please remove the 'stage' argument, see JDK-8325566

modules/javafx.controls/src/main/java/javafx/scene/control/Tooltip.java

bridgekeeper · 2024-05-11T00:30:42Z

@Maran23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

…tooltip-css

…orrect CSS processing for the Tooltip Node.

andy-goryachev-oracle · 2024-05-23T21:21:14Z

Strange: I still see the same issue described in
#1394 (comment)
(the very first tooltip after updating stylesheet uses the old value)

Maran23 · 2024-05-23T22:14:48Z

Strange: I still see the same issue described in #1394 (comment) (the very first tooltip after updating stylesheet uses the old value)

I don't know what is happening there, are you sure that there isn't a problem on your side? Testing Tooltip with a normal stylesheet added to the scene works perfectly fine for me.

andy-goryachev-oracle · 2024-05-23T22:23:41Z

I am not sure what the difference might be. The code in the MonkeyTester generates a stylesheet, encodes it in base64 data url and adds it to all scenes in the application (it also removes the old version, if any).

The stylesheet gets loaded - I can see it by slightly changed colors and the fact that the tooltip show delay gets changed on the second and any subsequent invocations.

Can you try it?
https://github.com/andy-goryachev-oracle/MonkeyTest

andy-goryachev-oracle · 2024-05-23T22:30:29Z

with the latest monkey tester I see that updating the stylesheet does not update the showingDelay property immediately. When the tooltip gets shown, I see the following output from the change listener added to this property:

showDelay=1000.0 ms
showDelay=100.0 ms

I suppose the second setting of 100ms (the value I actually set) happens too late or simply is ignored.

Maran23 · 2024-05-26T12:35:07Z

with the latest monkey tester I see that updating the stylesheet does not update the showingDelay property immediately. When the tooltip gets shown, I see the following output from the change listener added to this property:
showDelay=1000.0 ms
showDelay=100.0 ms
I suppose the second setting of 100ms (the value I actually set) happens too late or simply is ignored.

Ah, I think I know why. Same problem, we copy the stylesheet right before showing, so this is updated to late once again. We may need to do the update stuff always before showing.

…t matter

openjdk bot added the rfr Ready for review label Mar 8, 2024

Maran23 force-pushed the 8296387-tooltip-css branch from 6d40b52 to f0c5224 Compare March 8, 2024 16:03

Maran23 force-pushed the 8296387-tooltip-css branch from f0c5224 to cd9892b Compare March 8, 2024 16:04

JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the fir…

a4a1a49

…st tooltip that is shown before it is displayed

Maran23 force-pushed the 8296387-tooltip-css branch from cd9892b to a4a1a49 Compare March 8, 2024 16:05

hjohn reviewed Mar 9, 2024

View reviewed changes

Allow Tooltip to process the owner styles first so that also global s…

093b36f

…tylesheets are considered for the e.g. tooltip show-delay

Maran23 commented Mar 9, 2024

View reviewed changes

modules/javafx.controls/src/main/java/javafx/scene/control/Tooltip.java Outdated Show resolved Hide resolved

openjdk bot changed the title ~~JDK-8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed~~ 8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed Apr 10, 2024

andy-goryachev-oracle reviewed Apr 11, 2024

View reviewed changes

modules/javafx.controls/src/main/java/javafx/scene/control/Tooltip.java Outdated Show resolved Hide resolved

Maran23 added 2 commits May 23, 2024 16:36

Merge branch 'master' of https://github.com/openjdk/jfx into 8296387-…

d185aca

…tooltip-css

Implement applyStylesheetFromOwner(..) and use it instead to ensure c…

5af893c

…orrect CSS processing for the Tooltip Node.

Add more documentation and improve css stylesheet test threshold

a3f9f60

Add a test for changing the stylesheet and always process CSS for tha…

dc24b7f

…t matter

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed #1394

8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed #1394

Maran23 commented Mar 8, 2024 •

edited by openjdk bot

bridgekeeper bot commented Mar 8, 2024

mlbridge bot commented Mar 8, 2024 •

edited

openjdk bot commented Mar 8, 2024

openjdk bot commented Mar 8, 2024

openjdk bot commented Mar 8, 2024

andy-goryachev-oracle commented Mar 8, 2024

andy-goryachev-oracle commented Mar 8, 2024 •

edited

andy-goryachev-oracle commented Mar 8, 2024 •

edited

andy-goryachev-oracle commented Mar 8, 2024 •

edited

hjohn left a comment

Maran23 commented Mar 9, 2024

Maran23 commented Mar 9, 2024

Maran23 commented Mar 9, 2024 •

edited

openjdk bot commented Mar 13, 2024

bridgekeeper bot commented Apr 10, 2024

Maran23 commented Apr 10, 2024

andy-goryachev-oracle commented Apr 11, 2024 •

edited

bridgekeeper bot commented May 11, 2024

andy-goryachev-oracle commented May 23, 2024

Maran23 commented May 23, 2024

andy-goryachev-oracle commented May 23, 2024

andy-goryachev-oracle commented May 23, 2024

Maran23 commented May 26, 2024

8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed #1394

Are you sure you want to change the base?

8296387: [Tooltip, CSS] -fx-show-delay is only applied to the first tooltip that is shown before it is displayed #1394

Conversation

Maran23 commented Mar 8, 2024 • edited by openjdk bot

Progress

Issue

Reviewing

Webrev

bridgekeeper bot commented Mar 8, 2024

mlbridge bot commented Mar 8, 2024 • edited

Webrevs

openjdk bot commented Mar 8, 2024

openjdk bot commented Mar 8, 2024

openjdk bot commented Mar 8, 2024

andy-goryachev-oracle commented Mar 8, 2024

andy-goryachev-oracle commented Mar 8, 2024 • edited

andy-goryachev-oracle commented Mar 8, 2024 • edited

andy-goryachev-oracle commented Mar 8, 2024 • edited

hjohn left a comment

Choose a reason for hiding this comment

Maran23 commented Mar 9, 2024

Maran23 commented Mar 9, 2024

Maran23 commented Mar 9, 2024 • edited

openjdk bot commented Mar 13, 2024

bridgekeeper bot commented Apr 10, 2024

Maran23 commented Apr 10, 2024

andy-goryachev-oracle commented Apr 11, 2024 • edited

bridgekeeper bot commented May 11, 2024

andy-goryachev-oracle commented May 23, 2024

Maran23 commented May 23, 2024

andy-goryachev-oracle commented May 23, 2024

andy-goryachev-oracle commented May 23, 2024

Maran23 commented May 26, 2024

Maran23 commented Mar 8, 2024 •

edited by openjdk bot

mlbridge bot commented Mar 8, 2024 •

edited

andy-goryachev-oracle commented Mar 8, 2024 •

edited

andy-goryachev-oracle commented Mar 8, 2024 •

edited

andy-goryachev-oracle commented Mar 8, 2024 •

edited

Maran23 commented Mar 9, 2024 •

edited

andy-goryachev-oracle commented Apr 11, 2024 •

edited