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

[Andy Goryachev]

On Fri, 2 Jun 2023 19:18:13 GMT, Phil Race <prr at openjdk.org> wrote:

>> 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).
>
> No, it wasn't due to null text. That is provably impossible.
> 
> The exception from JDK-8302511 is as follows (copied from the bug report).
> 
> Exception in thread "JavaFX Application Thread" java.lang.IllegalArgumentException: offset out of bounds 
> at java.base/sun.text.RuleBasedBreakIterator.checkOffset(RuleBasedBreakIterator.java:730) 
> at java.base/sun.text.RuleBasedBreakIterator.following(RuleBasedBreakIterator.java:744) 
> at javafx.graphics/javafx.scene.text.HitInfo.getInsertionIndex(HitInfo.java:84) 
> at javafx.graphics/javafx.scene.text.HitInfo.toString(HitInfo.java:100) 
> Look at the code in HitInfo.java referenced in JDK-8302511,  the code at line 84 can only be reached if text != null
> ..
>               if (text != null) {
>                     // Skip complex character clusters / ligatures.
>                     int next;
>                     synchronized(charIterator) {
>                         charIterator.setText(text);
>                         next = charIterator.following(insertionIndex);   // this is line 84
>                     }
> 
> ...

you are right, it cannot be explained by a null text.

so if i understand you correctly, the question is about validity of `insertionIndex += 1` on lines 463 and 469.

463: should it be set to text.length() instead?

469: since text==null, could `insertionIndex = charIndex + 1` be correct?  

And also, is it possible at all that text == null within PrismTextLayout?

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

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