On Mon, 15 May 2023 15:50:06 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Initialize insertion index in PrismTextLayout
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
> line 479:
>
>> 477: insertionIndex = charIndex;
>> 478: if (!leading) {
>> 479: insertionIndex += 1;
>
> I would have initialized insertionIndex directly in all the blocks instead of
> adding conditional logic, but this code does seem to produce the desired
> result.
> For clarify, would it be possible to avoid this additional conditional logic?
>
> Also, do existing unit tests cover the newly modified paths?
Initializing the insertion index directly with character index looks better. In
this way we can avoid additional conditional logic as well. Changed code to do
so.
Added additional test case. This tests the condition present in line 430.
I couldn't create the scenario to hit line 473. If you have any suggestions
please let me know.
While testing I found that `testTextFlowInsertionIndexUsingMultipleEmojis()`
was failing intermittently when all the tests are run. Hence added small delay.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1194959394
