On Fri, 2 Jun 2023 07:19:01 GMT, Karthik P K <k...@openjdk.org> wrote:
>> 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. 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 ? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1214692324
