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

Karthik P K kpk at openjdk.org
Fri Jun 2 07:22:21 UTC 2023


On Thu, 1 Jun 2023 22:54:06 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Regarding
>> "If we can initialize insertionIndex to a positive value, then the buggy code in HitInfo.getInsertionIndex()
>> will never get executed, and we can remove it later (in JDK-8302511)."
>> 
>> it seems to me that this new code has been pretty much copied from the supposedly buggy code there .. 
>> I'm not sure that code is actually buggy (comments over in that bug), rather that the instance was constructed
>> with bad data.
>> 
>> But maybe this code here also needs to make sure it won't cause AIOB
>
> The main issue is that insertion index was not computed in the PrismTextLayout, supposedly to allow for delaying the computation until it's needed (if needed at all).  A good intention, but the execution had two issues:
> 1. HitInfo obtained in TextFlow had its 'text' field set to null (TextFlow:204), disabling any possibility of computing the index correctly
> 2. The code that computed the index had an issue in that if an exception happened, it left the object in an inconsistent state, leading to other problems down the road, and more exceptions (incl. AIOB)
> 
> What we did here is to compute the insertion index correctly at the source.
> I've also checked that in the case of an empty line the insertion index is correct in both cases (at the last line and in the middle of a document).

> Assuming line 473 is the one still there today, it looks to me as if that would be reached if you had the caret on an empty line that isn't the last line.

In this case also code hits the condition in 451 as the text run is not null.

>it seems to me that this new code has been pretty much copied from the supposedly buggy code there ..
I'm not sure that code is actually buggy (comments over in that bug), rather that the instance was constructed
with bad data.

Like Andy said, since text was null in `HitInfo::getInsertionIndex()` method in the case of TextFlow, correct insertion index was not calculated. I had to use pretty much same code in PrismTextLayout as I had to consider all the cases especially the U+FE0F Variation Selector-16 characters.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1214013961


More information about the openjfx-dev mailing list