On Tue, 1 Sep 2026 20:22:12 GMT, Andy Goryachev <[email protected]> wrote:

>> Fixes broken navigation introduced in 
>> [JDK-8370902](https://bugs.openjdk.org/browse/JDK-8370902) with non-zero 
>> paragraph spacing (above, below, line spacing).
>> 
>> Updated the Rich Editor Demo to allow setting paragraph line spacing in 
>> addition to space above/below (right click -> Paragraph).
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Andy Goryachev has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   review comments

Still a few more comments

modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/RangeInfo.java
 line 101:

> 99:         if (lines != null) {
> 100:             int sz = lines.length;
> 101:             if (y < lines[0]) {

You'd need to assert that sz > 0

modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/RangeInfo.java
 line 102:

> 100:             int sz = lines.length;
> 101:             if (y < lines[0]) {
> 102:                 return midPoint(0, lines[0]);

If I take it right, this would be a position in the space/top padding area, but 
the caret should go to the first line, therefore, it should be 
`midPoint(lines[0], lines[1])`?

modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/RangeInfo.java
 line 103:

> 101:             if (y < lines[0]) {
> 102:                 return midPoint(0, lines[0]);
> 103:             } else if (y >= lines[sz - 1]) {

make sure sz > 1

modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/RangeInfo.java
 line 104:

> 102:                 return midPoint(0, lines[0]);
> 103:             } else if (y >= lines[sz - 1]) {
> 104:                 return midPoint(lines[sz - 2], lines[sz - 1]);

(In this case, this is correct, as this is the middle of the last line, not the 
padding area)

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

PR Review: https://git.openjdk.org/jfx/pull/2280#pullrequestreview-5083487109
PR Review Comment: https://git.openjdk.org/jfx/pull/2280#discussion_r3908791562
PR Review Comment: https://git.openjdk.org/jfx/pull/2280#discussion_r3908806757
PR Review Comment: https://git.openjdk.org/jfx/pull/2280#discussion_r3908813271
PR Review Comment: https://git.openjdk.org/jfx/pull/2280#discussion_r3908816661

Reply via email to