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

[Karthik P K]

On Mon, 5 Jun 2023 15:46:00 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add out of bound check for insertion index
>
> tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java line 151:
> 
>> 149:     private void addMultipleEmojis() {
>> 150:         Util.runAndWait(() -> {
>> 151:             text = new Text("😊😇💙🦋🏁🔥");
> 
> Both new code (the last commit) and the old code (previous commit) fail this test if the text string is the following sequence:
> ☝🏿☝🏿☝🏿
> \u261d\ud83c\udfff\u261d\ud83c\udfff\u261d\ud83c\udfff
> 
> I've tried it with jdk19 and jdk20.0.1 (the latter contains a BreakInterator change [JDK-8291660](https://bugs.openjdk.org/browse/JDK-8291660) that alters its handling of grapheme clusters.
> 
> Exception:
> 
> java.lang.AssertionError: expected:<0> but was:<1>
> 	at org.junit.Assert.fail(Assert.java:89)
> 	at org.junit.Assert.failNotEquals(Assert.java:835)
> 	at org.junit.Assert.assertEquals(Assert.java:647)
> 	at org.junit.Assert.assertEquals(Assert.java:633)
> 	at test.robot.javafx.scene.TextFlowSurrogatePairInsertionIndexTest.testTextFlowInsertionIndexUsingMultipleEmojis(TextFlowSurrogatePairInsertionIndexTest.java:216)
> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
> 	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
> 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
> 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
> 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
> 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
> 	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
> 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
> 	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
> 	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
> 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
> 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
> 	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
> 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
> 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
> 	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
> 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)...

This case is same as the one which we saw with red heart as the emoji sequence mentioned here is also a U+FE0F Variation Selector-16 character. Since it exhibits different behavior in different platforms we decided to remove it.

I compared the insertion index calculated for these emojis with the insertion index calculated for red heart emoji. The insertion index calculated looks to be correct for the above given emojis. It is considered as 2 characters if color is also present else it is considered as single character.
For me it is giving correct caret position as well in the Monkey tester.

I found strange behavior when I tried to add heart emojis (with and without color) along with the `CLUSTERS` variable present in Monkey Tester so that it gets displayed when Rich Text option is selected.
When `CLUSTERS = "❤️❤❤️"`, emojis are displayed properly.
<img width="658" alt="image" src="https://github.com/openjdk/jfx/assets/26969459/f3de5d1d-ad8c-42a6-8284-01916dfd37a8">

When `CLUSTERS = "☝🏿☝🏿☝🏿🤦🏼‍♂️❤️❤❤️";`, following sequence is displayed.
<img width="532" alt="image" src="https://github.com/openjdk/jfx/assets/26969459/6eaa67b8-6fa1-475f-82bd-e9c48240b1c1">

This looks like a separate issue. Please let me know your thoughts on this.

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

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