On Fri, 2 Jun 2023 18:37:19 GMT, Phil Race <p...@openjdk.org> wrote:

>>> 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.
>
> I did note that TextFlow passed null which really looked like a WIP to me but 
> its a separate issue.
> The exception in 8302511 which is the entire subject of that bug isn't 
> possible if text == null.
> My point is that if you pass an incorrect charIndex to 
> BreakIterator.following() you'll get the same exception
> in your copy of that code. So how are you making sure you never do that ?

The exception in 8302511 was likely caused by a prior exception due to null 
text, which corrupted insertionIndex value in HitInfo.

The goal of this change is to always compute insertionIndex (an I believe we do 
it correctly this time).

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

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

Reply via email to