On Wed, 9 Jul 2025 15:28:08 GMT, Andy Goryachev <ango...@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

Reply via email to