On Mon, 8 May 2023 16:33:50 GMT, Andy Goryachev <ango...@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
