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

[Karthik P K]

On Mon, 8 May 2023 16:33:50 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> Does not seem to work correctly still - the insertion index in TextFlow should not point in between surrogate pair of characters that comprise an emoji. Basically, it should be exactly the same as with Text.hitInfo():
> 
This condition is working for me as `Text.hitInfo()`. The code is updated now to make the changes suggested above. Could you please recheck?

> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 456:
> 
>> 454: 
>> 455:                 insertionIndex = charIndex;
>> 456:                 String txt = new String(getText());
> 
> possible NPE: looks like getText() might return null, in which case this line will throw an NPE.

Updated code

> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 458:
> 
>> 456:                 String txt = new String(getText());
>> 457:                 if (!leading) {
>> 458:                     if (txt != null) {
> 
> This check should be done earlier for getText()

Added null check for `getText()`

> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 461:
> 
>> 459:                         int next;
>> 460:                         BreakIterator charIterator = BreakIterator.getCharacterInstance();
>> 461:                         synchronized(charIterator) {
> 
> question: why do we need to synchronize on the instance here?  BreakIterator.get*Instance() creates a new instance each time, so synchronization is probably unnecessary.

Correct it is not necessary. Updated code to remove synchronization

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

PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1539412625
PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1188134287
PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1188134476
PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1188134760