Skip to content

Fixes #1284 - JavaFX25 introduced getCaretShape to replace caretShape to fix#1285

Merged
Jugen merged 2 commits intoFXMisc:masterfrom
Symeon94:1284-jfx25-clashing-methods
Nov 19, 2025
Merged

Fixes #1284 - JavaFX25 introduced getCaretShape to replace caretShape to fix#1285
Jugen merged 2 commits intoFXMisc:masterfrom
Symeon94:1284-jfx25-clashing-methods

Conversation

@Symeon94
Copy link
Copy Markdown
Collaborator

@Symeon94 Symeon94 commented Aug 27, 2025

Fixes #1284 - My only concern with this change is that by removing the method, if someone extended TextFlowExt and used the method, it will suddenly hit the one from JavaFX 25 which is slightly different.

That being said, there is no real fix for that, as JavaFX 25 will not go away.

Copy link
Copy Markdown
Collaborator

@Jugen Jugen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also change hitTest which has been deprecated as well, to getHitInfo :

HitInfo hit = hitTest(new Point2D(x, y));

Comment thread richtextfx/src/main/java/org/fxmisc/richtext/ParagraphText.java
@Symeon94
Copy link
Copy Markdown
Collaborator Author

@Jugen for your comment about hitTest, we are not fixing deprecated methods here (and there might be risk of breaking support for older versions of JavaFX by doing so).
The instructions when pushing PR mentionned to only fix one problem per PR, I think, if we want to change the deprecated items we should proceed on a different PR.

@Jugen
Copy link
Copy Markdown
Collaborator

Jugen commented Aug 27, 2025

Removing getCaretShape is unnecessary then as it overrides the one in TextFlow.
The only reason we had to fix getUnderlineShape was because it was marked final, which is strangely not the case here ?

@Symeon94
Copy link
Copy Markdown
Collaborator Author

You are right about that, I didn't test the compilation on Java 25 and I assume the error from the comment was correct, but I don't see a final here.
That being said, it's not a good practice to override concrete methods (which only happens because JavaFX introduced the same method name as we have here).
Now, I totally see the problem of removing getCaretShape as suddenly a user that would override TextFlowExt and use that method would suddenly point to the one in TextFlow.
I think both solutions (removing/not removing it) are wrong, each in their way. But I tend to favor removing it.

That being said, I don't think there is any urge to merge this PR, so I'll let you decide what you want to do. I'm ready to modify it if you have any preference.

@Jugen
Copy link
Copy Markdown
Collaborator

Jugen commented Aug 27, 2025

I'll give it some thought, however I'm currently inclined to leave it as is.
Thanks for trying though, I appreciate the effort very much !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JFX25+: class org.fxmisc.richtext.TextFlowExt overrides final method javafx.scene.text.TextFlow.getUnderlineShape(II)[Ljavafx/scene/shape/PathElement;

2 participants