On Mon, 5 Jun 2023 11:16:52 GMT, Karthik P K <k...@openjdk.org> wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion index in `getHitInfo` method of
>> `PrismTextLayout` class when `hitTest` of trailing side of surrogate pair is
>> requested. Since text runs are processed in this method already, calculating
>> the insertion index in this method looks better than calculating in
>> `getInsertionIndex` of `HitInfo` method.
>> The latter approach also requires the text to be sent to `HitInfo` as
>> parameter from the `hitTest` method of `TextFlow`. If the number of `Text`
>> nodes in `TextFlow` are very large, processing all the `Text` nodes on each
>> `hitTest` method invocation might cause performance issue. Hence implemented
>> first approach.
>>
>> Added system test to validate the fix.
>
> 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
The new code also exhibits a strange behavior in the RichTextArea (based ob
TextFlow): I am unable to position the cursor at the end of ☝🏿☝🏿☝🏿 emoji
sequence with the mouse (unless I click on the next empty line).
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)
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.ParentRunner.run(ParentRunner.java:413)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
at
org.junit.vintage.engine.execution.RunnerExecutor.execute(RunnerExecutor.java:42)
at
org.junit.vintage.engine.VintageTestEngine.executeAllChildren(VintageTestEngine.java:80)
at
org.junit.vintage.engine.VintageTestEngine.execute(VintageTestEngine.java:72)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
at
org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:95)
at
org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:91)
at
org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:60)
at
org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
at
org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1577099109
PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1218267510
