RFR: 8304831: TextFlow.hitTest.insertionIndex incorrect with surrogate pairs [v4]

Karthik P K kpk at openjdk.org
Tue May 16 10:34:54 UTC 2023


On Mon, 15 May 2023 15:50:06 GMT, Andy Goryachev <angorya at 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


More information about the openjfx-dev mailing list