RFR: 8341438: TextFlow: incorrect caretShape(), hitTest(), rangeShape() with non-empty padding/border [v3]

Kevin Rushforth kcr at openjdk.org
Wed Jul 9 20:23:48 UTC 2025


On Wed, 9 Jul 2025 15:28:08 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> ## Summary
>> This change adds new methods to the `TextFlow` which work correctly in the presence of non-empty insets (borders/padding). For backward compatibility, the old buggy methods are getting deprecated (not for removal). Also, adds new methods in Text which provide missing functionality.
>> 
>> ## Problem
>> A number of methods in `TextFlow` fail to return correct values in the presence of non-empty insets (i.e. when either padding or border are set):
>> - caretShape 
>> - hitTest 
>> - rangeShape
>> 
>> Additionally, the current API fail to provide strike-through shape, and account for line spacing in the range shape, a problem shared between the `TextFlow` and the `Text` classes.
>> 
>> ## Solution
>> The solution is two-fold: 
>> 1) deprecate the buggy methods (not for removal), adding explanations in their javadoc comments 
>> 2) add new methods that behave correctly in the presence of non-empty insets and/or implementing the missing functionality.
>> 
>> The proposed solution retains the buggy methods for the purposes of backward compatibility in applications which employ the workarounds, while providing new APIs with additional parameters similar to those offered by the new TextLayout API https://bugs.openjdk.org/browse/JDK-8341670
>> 
>> ## Testing
>> 
>> Additional visualization of the data returned by the new APIs is available in the Monkey Tester using the following branch (in the Text and TextFlow pages):
>> 
>> https://github.com/andy-goryachev-oracle/MonkeyTest/tree/text.insets.corrected
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 72 commits:
> 
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8341438.text.shapes.insets
>  - Merge branch 'master' into 8341438.text.shapes.insets
>  - cleanup
>  - Merge branch 'master' into 8341438.text.shapes.insets
>  - layout info
>  - tests
>  - Merge remote-tracking branch 'origin/ag.text.layout.api' into 8341438.text.shapes.insets
>  - cleanup
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - ... and 62 more: https://git.openjdk.org/jfx/compare/639a5950...16ed27eb

The API and docs look good to me now. I left a minor formatting suggestion for the deprecated methods (use `@link` to point to the replacement methods).

Update the CSR to match, and I'll Review it.

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 202:

> 200:      * @return a {@code HitInfo} representing the character index found
> 201:      * @since 9
> 202:      * @deprecated replaced by {@code getHitInfo()}

I recommend `@link` here.

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 245:

> 243:      * @return an array of {@code PathElement} which can be used to create a {@code Shape}
> 244:      * @since 9
> 245:      * @deprecated replaced by {@code getCaretShape()}

`@link` (here and in. other deprecated methods).

-------------

PR Review: https://git.openjdk.org/jfx/pull/1817#pullrequestreview-3002944702
PR Review Comment: https://git.openjdk.org/jfx/pull/1817#discussion_r2195889785
PR Review Comment: https://git.openjdk.org/jfx/pull/1817#discussion_r2195894529


More information about the openjfx-dev mailing list