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