From f741febcbcd1be20cce054ae18b3652b7c5214b0 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Fri, 14 Apr 2023 12:40:19 +0530 Subject: [PATCH 1/8] Fix insertion index calculation issue in TextFlow --- .../com/sun/javafx/text/PrismTextLayout.java | 13 +- ...xtFlowSurrogatePairInsertionIndexTest.java | 170 ++++++++++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 71d76a21234..f100ad5334a 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -422,6 +422,7 @@ public PathElement[] getCaretShape(int offset, boolean isLeading, @Override public Hit getHitInfo(float x, float y) { int charIndex = -1; + int insertionIndex = -1; boolean leading = false; ensureLayout(); @@ -450,13 +451,23 @@ public Hit getHitInfo(float x, float y) { int[] trailing = new int[1]; charIndex = run.getStart() + run.getOffsetAtX(x, trailing); leading = (trailing[0] == 0); + + // Calculate insertion index for surrogate pair text in TextFlow + // when leading is false + if (run.getTextSpan() != null) { + String txt = run.getTextSpan().getText(); + int relIdx = charIndex - run.getStart(); + if (Character.isHighSurrogate(txt.charAt(relIdx)) && !leading) { + insertionIndex = charIndex + 2; + } + } } else { //empty line, set to line break leading charIndex = line.getStart(); leading = true; } } - return new Hit(charIndex, -1, leading); + return new Hit(charIndex, insertionIndex, leading); } @Override diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java new file mode 100644 index 00000000000..67e503b5e83 --- /dev/null +++ b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.robot.javafx.scene; + +import java.util.concurrent.CountDownLatch; + +import javafx.application.Application; +import javafx.application.Platform; +import javafx.geometry.Point2D; +import javafx.scene.Scene; +import javafx.scene.input.MouseEvent; +import javafx.scene.input.MouseButton; +import javafx.scene.robot.Robot; +import javafx.scene.text.Font; +import javafx.scene.text.HitInfo; +import javafx.scene.text.Text; +import javafx.scene.text.TextFlow; +import javafx.stage.Stage; +import javafx.stage.StageStyle; +import javafx.stage.WindowEvent; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.sun.javafx.PlatformUtil; +import com.sun.javafx.tk.Toolkit; + +import test.util.Util; + +/* + * Test for verifying insertion index in TextFlow surrogate pairs + * + * There is 1 test in this file. + * Steps for testTextFlowSurrogatePairInsertionIndex() + * 1. Create a TextFlow. Add Text node with surrogate pair to it. + * 2. Move the cursor and click on the leading side of a surrogate pair text. + * 3. Insertion index should be same as character index. + * 4. Move the cursor and click on the trailing side of a surrogate pair text. + * 5. Insertion index should 2 more than the character index. + */ + +public class TextFlowSurrogatePairInsertionIndexTest { + static CountDownLatch startupLatch = new CountDownLatch(1); + static Robot robot; + static TextFlow textFlow; + + static volatile Stage stage; + static volatile Scene scene; + + boolean isLeading; + int charIndex; + int insertionIndex; + + CountDownLatch mouseClickLatch; + + private void mouseClick(double x, double y) { + Util.runAndWait(() -> { + robot.mouseMove((int) (scene.getWindow().getX() + scene.getX() + x), + (int) (scene.getWindow().getY() + scene.getY() + y)); + robot.mousePress(MouseButton.PRIMARY); + robot.mouseRelease(MouseButton.PRIMARY); + mouseClickLatch.countDown(); + }); + } + + private void moveMouseToLeadingSide() throws Exception { + mouseClickLatch = new CountDownLatch(1); + mouseClick(textFlow.getLayoutX() + (textFlow.getWidth() / 4) - 10, + textFlow.getLayoutY() + textFlow.getHeight() / 2); + Thread.sleep(200); // Small delay to wait for cursor to move and mouse click. + Util.waitForLatch(mouseClickLatch, 10, "Timeout waiting for mouse click"); + } + + private void moveMouseToTrailingSide() throws Exception { + mouseClickLatch = new CountDownLatch(1); + mouseClick(textFlow.getLayoutX() + (textFlow.getWidth() / 4) + 10, + textFlow.getLayoutY() + textFlow.getHeight() / 2); + Thread.sleep(200); // Small delay to wait for cursor to move and mouse click. + Util.waitForLatch(mouseClickLatch, 10, "Timeout waiting for mouse click"); + } + + @Test + public void testTextFlowSurrogatePairInsertionIndex() throws Exception { + moveMouseToLeadingSide(); + Assert.assertTrue(isLeading); + Assert.assertEquals(charIndex, insertionIndex); + + moveMouseToTrailingSide(); + Assert.assertFalse(isLeading); + Assert.assertEquals(charIndex, insertionIndex - 2); + } + + private void handleMouseEvent(MouseEvent event) { + Point2D point = new Point2D(event.getX(), event.getY()); + HitInfo hitInfo = textFlow.hitTest(point); + isLeading = hitInfo.isLeading(); + charIndex = hitInfo.getCharIndex(); + insertionIndex = hitInfo.getInsertionIndex(); + } + + @After + public void resetUI() { + Platform.runLater(() -> { + textFlow.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + }); + } + + @Before + public void setupUI() { + Platform.runLater(() -> { + textFlow.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + }); + } + + @BeforeClass + public static void initFX() { + Util.launch(startupLatch, TestApp.class); + } + + @AfterClass + public static void exit() { + Util.shutdown(stage); + } + + public static class TestApp extends Application { + @Override + public void start(Stage primaryStage) { + robot = new Robot(); + stage = primaryStage; + + Text text = new Text("😊😇"); + text.setFont(new Font(48)); + textFlow = new TextFlow(text); + + scene = new Scene(textFlow); + stage.setScene(scene); + stage.initStyle(StageStyle.UNDECORATED); + stage.setOnShown(event -> Platform.runLater(startupLatch::countDown)); + stage.setAlwaysOnTop(true); + stage.show(); + } + } +} From e35f5bf61b98650a39f561c7dd6f9f7f5c6bedd2 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 8 May 2023 17:47:47 +0530 Subject: [PATCH 2/8] Fix insertion index calculation issue with emojis. Add additional test cases --- .../com/sun/javafx/text/PrismTextLayout.java | 24 ++- ...xtFlowSurrogatePairInsertionIndexTest.java | 179 ++++++++++++++++-- 2 files changed, 177 insertions(+), 26 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index f100ad5334a..b113fb3aec1 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -452,13 +452,23 @@ public Hit getHitInfo(float x, float y) { charIndex = run.getStart() + run.getOffsetAtX(x, trailing); leading = (trailing[0] == 0); - // Calculate insertion index for surrogate pair text in TextFlow - // when leading is false - if (run.getTextSpan() != null) { - String txt = run.getTextSpan().getText(); - int relIdx = charIndex - run.getStart(); - if (Character.isHighSurrogate(txt.charAt(relIdx)) && !leading) { - insertionIndex = charIndex + 2; + insertionIndex = charIndex; + String txt = new String(getText()); + if (!leading) { + if (txt != null) { + int next; + BreakIterator charIterator = BreakIterator.getCharacterInstance(); + synchronized(charIterator) { + charIterator.setText(txt); + next = charIterator.following(insertionIndex); + } + if (next == BreakIterator.DONE) { + insertionIndex += 1; + } else { + insertionIndex = next; + } + } else { + insertionIndex += 1; } } } else { diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java index 67e503b5e83..551e129091e 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java @@ -40,7 +40,6 @@ import javafx.scene.text.TextFlow; import javafx.stage.Stage; import javafx.stage.StageStyle; -import javafx.stage.WindowEvent; import org.junit.After; import org.junit.AfterClass; @@ -57,23 +56,51 @@ /* * Test for verifying insertion index in TextFlow surrogate pairs * - * There is 1 test in this file. - * Steps for testTextFlowSurrogatePairInsertionIndex() + * There are 4 tests in this file. + * Steps for testTextFlowInsertionIndexUsingTwoEmojis() * 1. Create a TextFlow. Add Text node with surrogate pair to it. * 2. Move the cursor and click on the leading side of a surrogate pair text. * 3. Insertion index should be same as character index. * 4. Move the cursor and click on the trailing side of a surrogate pair text. * 5. Insertion index should 2 more than the character index. + * + * Steps for testTextFlowInsertionIndexUsingMultipleEmojis() + * 1. Create a TextFlow. Add Text node with multiple emojis (surrogate pairs). + * 2. Move the cursor to the first character and click. + * 3. Insertion index should be same as character index. + * 4. Move the cursor continously till last character and check that + * character index and insertion index increase monitonically as expected. + * + * Steps for testTextFlowInsertionIndexUsingTextAndEmojis() + * 1. Create a TextFlow. Add Text node with multiple emojis (surrogate pairs). + * 2. Move the cursor to the first character and click. + * 3. Insertion index should be same as character index. + * 4. Move the cursor continously till last character and check that + * character index and insertion index increase monitonically as expected. + * + * Steps for testTextFlowInsertionIndexUsingEmbeddedTextNodes() + * 1. Create a TextFlow. Add a Text node with text and another with emojis. + * 2. Move the cursor to the first character and click. + * 3. Insertion index should be same as character index. + * 4. Move the cursor continously till last character and check that + * character index and insertion index increase monitonically as expected. */ public class TextFlowSurrogatePairInsertionIndexTest { static CountDownLatch startupLatch = new CountDownLatch(1); + static CountDownLatch textSetLatch; static Robot robot; static TextFlow textFlow; + static Text text; + static Text emoji; static volatile Stage stage; static volatile Scene scene; + final int Y_OFFSET = 25; + final int X_LEADING_OFFSET = 10; + final int X_TRAILING_OFFSET = 40; + boolean isLeading; int charIndex; int insertionIndex; @@ -84,30 +111,91 @@ private void mouseClick(double x, double y) { Util.runAndWait(() -> { robot.mouseMove((int) (scene.getWindow().getX() + scene.getX() + x), (int) (scene.getWindow().getY() + scene.getY() + y)); - robot.mousePress(MouseButton.PRIMARY); - robot.mouseRelease(MouseButton.PRIMARY); + robot.mouseClick(MouseButton.PRIMARY); mouseClickLatch.countDown(); }); } private void moveMouseToLeadingSide() throws Exception { mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + (textFlow.getWidth() / 4) - 10, - textFlow.getLayoutY() + textFlow.getHeight() / 2); - Thread.sleep(200); // Small delay to wait for cursor to move and mouse click. - Util.waitForLatch(mouseClickLatch, 10, "Timeout waiting for mouse click"); + mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET, + textFlow.getLayoutY() + Y_OFFSET); + Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); } private void moveMouseToTrailingSide() throws Exception { mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + (textFlow.getWidth() / 4) + 10, - textFlow.getLayoutY() + textFlow.getHeight() / 2); - Thread.sleep(200); // Small delay to wait for cursor to move and mouse click. - Util.waitForLatch(mouseClickLatch, 10, "Timeout waiting for mouse click"); + mouseClick(textFlow.getLayoutX() + X_TRAILING_OFFSET, + textFlow.getLayoutY() + Y_OFFSET); + Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); + } + + private void moveMouseByPixel(int c) throws Exception { + mouseClickLatch = new CountDownLatch(1); + mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET + c, + textFlow.getLayoutY() + Y_OFFSET); + Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); + } + + private void addTwoEmojis() { + textSetLatch = new CountDownLatch(1); + Util.runAndWait(() -> { + text = new Text("😊😇"); + text.setFont(new Font(48)); + textFlow.getChildren().clear(); + textFlow.getChildren().setAll(text); + + textSetLatch.countDown(); + }); + Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); + } + + private void addMultipleEmojis() { + textSetLatch = new CountDownLatch(1); + Util.runAndWait(() -> { + text = new Text("😊😇❤️💙🦋🏁🔥"); + text.setFont(new Font(48)); + textFlow.getChildren().clear(); + textFlow.getChildren().setAll(text); + + textSetLatch.countDown(); + }); + Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); + } + + private void addTextAndEmojis() { + textSetLatch = new CountDownLatch(1); + Util.runAndWait(() -> { + text = new Text("Text 😊😇❤️💙🦋🔥"); + text.setFont(new Font(48)); + textFlow.getChildren().clear(); + textFlow.getChildren().setAll(text); + + textSetLatch.countDown(); + }); + Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); + } + + private void addTwoTextNodes() { + textSetLatch = new CountDownLatch(1); + Util.runAndWait(() -> { + text = new Text("Text"); + text.setFont(new Font(48)); + + emoji = new Text("😊😇"); + emoji.setFont(new Font(48)); + + textFlow.getChildren().clear(); + textFlow.getChildren().setAll(text, emoji); + + textSetLatch.countDown(); + }); + Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } @Test - public void testTextFlowSurrogatePairInsertionIndex() throws Exception { + public void testTextFlowInsertionIndexUsingTwoEmojis() throws Exception { + addTwoEmojis(); moveMouseToLeadingSide(); Assert.assertTrue(isLeading); Assert.assertEquals(charIndex, insertionIndex); @@ -117,6 +205,62 @@ public void testTextFlowSurrogatePairInsertionIndex() throws Exception { Assert.assertEquals(charIndex, insertionIndex - 2); } + @Test + public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { + addMultipleEmojis(); + + int textLength = text.getText().length(); + int index = 0; + while (charIndex < textLength - 2) { + moveMouseByPixel(index); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 2); + } + index += 5; + } + } + + @Test + public void testTextFlowInsertionIndexUsingTextAndEmojis() throws Exception { + addTextAndEmojis(); + + int textLength = text.getText().length(); + int index = 0; + while (charIndex < textLength - 2) { + moveMouseByPixel(index); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else if (!isLeading && charIndex < 5) { + Assert.assertEquals(charIndex, insertionIndex - 1); + } else { + Assert.assertEquals(charIndex, insertionIndex - 2); + } + index += 5; + } + } + + @Test + public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception { + addTwoTextNodes(); + + int textLength = text.getText().length(); + textLength += emoji.getText().length(); + int index = 0; + while (charIndex < textLength - 2) { + moveMouseByPixel(index); + if(isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else if (!isLeading && charIndex < 4) { + Assert.assertEquals(charIndex, insertionIndex - 1); + } else { + Assert.assertEquals(charIndex, insertionIndex - 2); + } + index += 5; + } + } + private void handleMouseEvent(MouseEvent event) { Point2D point = new Point2D(event.getX(), event.getY()); HitInfo hitInfo = textFlow.hitTest(point); @@ -155,11 +299,8 @@ public void start(Stage primaryStage) { robot = new Robot(); stage = primaryStage; - Text text = new Text("😊😇"); - text.setFont(new Font(48)); - textFlow = new TextFlow(text); - - scene = new Scene(textFlow); + textFlow = new TextFlow(); + scene = new Scene(textFlow, 500, 100); stage.setScene(scene); stage.initStyle(StageStyle.UNDECORATED); stage.setOnShown(event -> Platform.runLater(startupLatch::countDown)); From 036005527ea600e81ea8c772013970d28c5dcad0 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 9 May 2023 10:32:10 +0530 Subject: [PATCH 3/8] Address code review --- .../com/sun/javafx/text/PrismTextLayout.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index b113fb3aec1..b71d4af902f 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -453,23 +453,19 @@ public Hit getHitInfo(float x, float y) { leading = (trailing[0] == 0); insertionIndex = charIndex; - String txt = new String(getText()); - if (!leading) { - if (txt != null) { - int next; + if (getText() != null) { + if (!leading) { BreakIterator charIterator = BreakIterator.getCharacterInstance(); - synchronized(charIterator) { - charIterator.setText(txt); - next = charIterator.following(insertionIndex); - } + charIterator.setText(new String(getText())); + int next = charIterator.following(insertionIndex); if (next == BreakIterator.DONE) { insertionIndex += 1; } else { insertionIndex = next; } - } else { - insertionIndex += 1; } + } else if (!leading) { + insertionIndex += 1; } } else { //empty line, set to line break leading From 7919d5b562e63a2ccb98e0a65fabc93ea8d101ba Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 15 May 2023 15:21:08 +0530 Subject: [PATCH 4/8] Initialize insertion index in PrismTextLayout --- .../src/main/java/com/sun/javafx/text/PrismTextLayout.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index b71d4af902f..337a63bc299 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -473,6 +473,12 @@ public Hit getHitInfo(float x, float y) { leading = true; } } + if (insertionIndex == -1) { + insertionIndex = charIndex; + if (!leading) { + insertionIndex += 1; + } + } return new Hit(charIndex, insertionIndex, leading); } From 1eb11749ed4b5d983c624e8c0e832474be5e6cfc Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 16 May 2023 15:56:35 +0530 Subject: [PATCH 5/8] Change insertion index initialization approach. Add additional test --- .../com/sun/javafx/text/PrismTextLayout.java | 8 ++----- ...xtFlowSurrogatePairInsertionIndexTest.java | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 337a63bc299..5588849c711 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -429,6 +429,7 @@ public Hit getHitInfo(float x, float y) { int lineIndex = getLineIndex(y); if (lineIndex >= getLineCount()) { charIndex = getCharCount(); + insertionIndex = charIndex + 1; } else { if (isMirrored()) { x = getMirroringWidth() - x; @@ -471,12 +472,7 @@ public Hit getHitInfo(float x, float y) { //empty line, set to line break leading charIndex = line.getStart(); leading = true; - } - } - if (insertionIndex == -1) { - insertionIndex = charIndex; - if (!leading) { - insertionIndex += 1; + insertionIndex = charIndex; } } return new Hit(charIndex, insertionIndex, leading); diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java index 551e129091e..4755aec89ae 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java @@ -137,6 +137,13 @@ private void moveMouseByPixel(int c) throws Exception { Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); } + private void moveMouseVertically(int x, int y) throws Exception { + mouseClickLatch = new CountDownLatch(1); + mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET + x, + textFlow.getLayoutY() + Y_OFFSET + y); + Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); + } + private void addTwoEmojis() { textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { @@ -211,6 +218,7 @@ public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { int textLength = text.getText().length(); int index = 0; + Thread.sleep(500); while (charIndex < textLength - 2) { moveMouseByPixel(index); if (isLeading) { @@ -261,6 +269,22 @@ public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception } } + @Test + public void testTextFlowInsertionIndexWhenMouseMovedOutsideText() throws Exception { + addTextAndEmojis(); + + int index = 0; + while (index < (2 * Y_OFFSET)) { + moveMouseVertically(X_LEADING_OFFSET, index); + if(isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + index += 5; + } + } + private void handleMouseEvent(MouseEvent event) { Point2D point = new Point2D(event.getX(), event.getY()); HitInfo hitInfo = textFlow.hitTest(point); From a43bbdb29b72c222fee14e8f92f93f1a5d9edad1 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Thu, 18 May 2023 14:52:26 +0530 Subject: [PATCH 6/8] Add new test. Optimizations to make the tests robust --- ...xtFlowSurrogatePairInsertionIndexTest.java | 122 ++++++++++++------ 1 file changed, 84 insertions(+), 38 deletions(-) diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java index 4755aec89ae..522f6c7e4db 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java @@ -84,6 +84,22 @@ * 3. Insertion index should be same as character index. * 4. Move the cursor continously till last character and check that * character index and insertion index increase monitonically as expected. + * + * Steps for testTextFlowInsertionIndexWhenMouseMovedOutsideText() + * 1. Create a TextFlow. Add a Text node with text and emojis. + * 2. Move the cursor to the first character and click. + * 3. Insertion index should be same as character index. + * 4. Move the cursor towards bottom of the application window check that + * chracter index and insertion index are as expected. + * This test is implemented to test insertion index initialization + * when text run is not used to calculate character index. + * + * Steps for testTextFlowInsertionIndexUsingWrappedText() + * 1. Create a TextFlow. Add a Text node with text and emojis whose length is + * more than the size of application window. + * 2. Move the cursor from first character to last character. + * 3. Character index should increase monotonically as expected. + * 4. Insertion index should also increase as expected along with character index. */ public class TextFlowSurrogatePairInsertionIndexTest { @@ -97,11 +113,15 @@ public class TextFlowSurrogatePairInsertionIndexTest { static volatile Stage stage; static volatile Scene scene; + static final int WIDTH = 500; + static final int HEIGHT = 150; + final int Y_OFFSET = 25; final int X_LEADING_OFFSET = 10; final int X_TRAILING_OFFSET = 40; boolean isLeading; + boolean isSurrogatePair; int charIndex; int insertionIndex; @@ -116,31 +136,10 @@ private void mouseClick(double x, double y) { }); } - private void moveMouseToLeadingSide() throws Exception { - mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET, - textFlow.getLayoutY() + Y_OFFSET); - Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); - } - - private void moveMouseToTrailingSide() throws Exception { - mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + X_TRAILING_OFFSET, - textFlow.getLayoutY() + Y_OFFSET); - Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); - } - - private void moveMouseByPixel(int c) throws Exception { + private void moveMouseOverTextFlow(int x, int y) throws Exception { mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET + c, - textFlow.getLayoutY() + Y_OFFSET); - Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); - } - - private void moveMouseVertically(int x, int y) throws Exception { - mouseClickLatch = new CountDownLatch(1); - mouseClick(textFlow.getLayoutX() + X_LEADING_OFFSET + x, - textFlow.getLayoutY() + Y_OFFSET + y); + mouseClick(textFlow.getLayoutX() + x, + textFlow.getLayoutY() + y); Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); } @@ -200,14 +199,30 @@ private void addTwoTextNodes() { Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } + private void addLongText() { + textSetLatch = new CountDownLatch(1); + Util.runAndWait(() -> { + text = new Text("[This is text 😀😃😄😁😆 🙂🙃😉😊😇]"); + text.setFont(new Font(48)); + + textFlow.getChildren().clear(); + textFlow.getChildren().setAll(text); + + textSetLatch.countDown(); + }); + Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); + } + @Test public void testTextFlowInsertionIndexUsingTwoEmojis() throws Exception { addTwoEmojis(); - moveMouseToLeadingSide(); + Thread.sleep(500); + + moveMouseOverTextFlow(X_LEADING_OFFSET, Y_OFFSET); Assert.assertTrue(isLeading); Assert.assertEquals(charIndex, insertionIndex); - moveMouseToTrailingSide(); + moveMouseOverTextFlow(X_TRAILING_OFFSET, Y_OFFSET); Assert.assertFalse(isLeading); Assert.assertEquals(charIndex, insertionIndex - 2); } @@ -215,12 +230,12 @@ public void testTextFlowInsertionIndexUsingTwoEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { addMultipleEmojis(); + Thread.sleep(500); int textLength = text.getText().length(); int index = 0; - Thread.sleep(500); while (charIndex < textLength - 2) { - moveMouseByPixel(index); + moveMouseOverTextFlow(index, Y_OFFSET); if (isLeading) { Assert.assertEquals(charIndex, insertionIndex); } else { @@ -233,11 +248,12 @@ public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingTextAndEmojis() throws Exception { addTextAndEmojis(); + Thread.sleep(500); int textLength = text.getText().length(); int index = 0; while (charIndex < textLength - 2) { - moveMouseByPixel(index); + moveMouseOverTextFlow(index, Y_OFFSET); if (isLeading) { Assert.assertEquals(charIndex, insertionIndex); } else if (!isLeading && charIndex < 5) { @@ -252,18 +268,19 @@ public void testTextFlowInsertionIndexUsingTextAndEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception { addTwoTextNodes(); + Thread.sleep(500); int textLength = text.getText().length(); textLength += emoji.getText().length(); int index = 0; while (charIndex < textLength - 2) { - moveMouseByPixel(index); - if(isLeading) { + moveMouseOverTextFlow(index, Y_OFFSET); + if (isLeading) { Assert.assertEquals(charIndex, insertionIndex); - } else if (!isLeading && charIndex < 4) { - Assert.assertEquals(charIndex, insertionIndex - 1); - } else { + } else if (isSurrogatePair) { Assert.assertEquals(charIndex, insertionIndex - 2); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); } index += 5; } @@ -272,11 +289,12 @@ public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception @Test public void testTextFlowInsertionIndexWhenMouseMovedOutsideText() throws Exception { addTextAndEmojis(); + Thread.sleep(500); int index = 0; - while (index < (2 * Y_OFFSET)) { - moveMouseVertically(X_LEADING_OFFSET, index); - if(isLeading) { + while (index < (HEIGHT - Y_OFFSET)) { + moveMouseOverTextFlow(X_LEADING_OFFSET, (Y_OFFSET + index)); + if (isLeading) { Assert.assertEquals(charIndex, insertionIndex); } else { Assert.assertEquals(charIndex, insertionIndex - 1); @@ -285,12 +303,40 @@ public void testTextFlowInsertionIndexWhenMouseMovedOutsideText() throws Excepti } } + @Test + public void testTextFlowInsertionIndexUsingWrappedText() throws Exception { + addLongText(); + Thread.sleep(500); + + for (int y = 0; y < 2; y++) { + for (int x = 0; x < (WIDTH - X_LEADING_OFFSET); x += 5) { + moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else if (isSurrogatePair) { + Assert.assertEquals(charIndex, insertionIndex - 2); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + } + } + } + private void handleMouseEvent(MouseEvent event) { Point2D point = new Point2D(event.getX(), event.getY()); HitInfo hitInfo = textFlow.hitTest(point); isLeading = hitInfo.isLeading(); charIndex = hitInfo.getCharIndex(); insertionIndex = hitInfo.getInsertionIndex(); + + String testString = text.getText(); + if (charIndex >= testString.length() && emoji != null) { + testString += emoji.getText(); + } + if (charIndex < testString.length()) { + char c = testString.charAt(charIndex); + isSurrogatePair = Character.isSurrogate(c); + } } @After @@ -324,7 +370,7 @@ public void start(Stage primaryStage) { stage = primaryStage; textFlow = new TextFlow(); - scene = new Scene(textFlow, 500, 100); + scene = new Scene(textFlow, WIDTH, HEIGHT); stage.setScene(scene); stage.initStyle(StageStyle.UNDECORATED); stage.setOnShown(event -> Platform.runLater(startupLatch::countDown)); From 6684075e36eac93ace1c9bd175f2629df6f55728 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 22 May 2023 17:06:01 +0530 Subject: [PATCH 7/8] Stabilizing the system tests --- ...xtFlowSurrogatePairInsertionIndexTest.java | 42 ++++--------------- .../system/src/test/java/test/util/Util.java | 33 +++++++++++++++ 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java index 522f6c7e4db..d2b71c96f20 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java @@ -104,7 +104,6 @@ public class TextFlowSurrogatePairInsertionIndexTest { static CountDownLatch startupLatch = new CountDownLatch(1); - static CountDownLatch textSetLatch; static Robot robot; static TextFlow textFlow; static Text text; @@ -125,65 +124,47 @@ public class TextFlowSurrogatePairInsertionIndexTest { int charIndex; int insertionIndex; - CountDownLatch mouseClickLatch; - private void mouseClick(double x, double y) { Util.runAndWait(() -> { robot.mouseMove((int) (scene.getWindow().getX() + scene.getX() + x), (int) (scene.getWindow().getY() + scene.getY() + y)); robot.mouseClick(MouseButton.PRIMARY); - mouseClickLatch.countDown(); }); } private void moveMouseOverTextFlow(int x, int y) throws Exception { - mouseClickLatch = new CountDownLatch(1); mouseClick(textFlow.getLayoutX() + x, textFlow.getLayoutY() + y); - Util.waitForLatch(mouseClickLatch, 5, "Timeout waiting for mouse click"); } private void addTwoEmojis() { - textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { text = new Text("😊😇"); text.setFont(new Font(48)); textFlow.getChildren().clear(); textFlow.getChildren().setAll(text); - - textSetLatch.countDown(); }); - Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } private void addMultipleEmojis() { - textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { - text = new Text("😊😇❤️💙🦋🏁🔥"); + text = new Text("😊😇💙🦋🏁🔥"); text.setFont(new Font(48)); textFlow.getChildren().clear(); textFlow.getChildren().setAll(text); - - textSetLatch.countDown(); }); - Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } private void addTextAndEmojis() { - textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { - text = new Text("Text 😊😇❤️💙🦋🔥"); + text = new Text("Text 😊😇💙🦋🔥"); text.setFont(new Font(48)); textFlow.getChildren().clear(); textFlow.getChildren().setAll(text); - - textSetLatch.countDown(); }); - Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } private void addTwoTextNodes() { - textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { text = new Text("Text"); text.setFont(new Font(48)); @@ -193,30 +174,23 @@ private void addTwoTextNodes() { textFlow.getChildren().clear(); textFlow.getChildren().setAll(text, emoji); - - textSetLatch.countDown(); }); - Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } private void addLongText() { - textSetLatch = new CountDownLatch(1); Util.runAndWait(() -> { text = new Text("[This is text 😀😃😄😁😆 🙂🙃😉😊😇]"); text.setFont(new Font(48)); textFlow.getChildren().clear(); textFlow.getChildren().setAll(text); - - textSetLatch.countDown(); }); - Util.waitForLatch(textSetLatch, 5, "Timeout waiting for text intialization."); } @Test public void testTextFlowInsertionIndexUsingTwoEmojis() throws Exception { addTwoEmojis(); - Thread.sleep(500); + Util.waitForIdle(scene); moveMouseOverTextFlow(X_LEADING_OFFSET, Y_OFFSET); Assert.assertTrue(isLeading); @@ -230,7 +204,7 @@ public void testTextFlowInsertionIndexUsingTwoEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { addMultipleEmojis(); - Thread.sleep(500); + Util.waitForIdle(scene); int textLength = text.getText().length(); int index = 0; @@ -248,7 +222,7 @@ public void testTextFlowInsertionIndexUsingMultipleEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingTextAndEmojis() throws Exception { addTextAndEmojis(); - Thread.sleep(500); + Util.waitForIdle(scene); int textLength = text.getText().length(); int index = 0; @@ -268,7 +242,7 @@ public void testTextFlowInsertionIndexUsingTextAndEmojis() throws Exception { @Test public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception { addTwoTextNodes(); - Thread.sleep(500); + Util.waitForIdle(scene); int textLength = text.getText().length(); textLength += emoji.getText().length(); @@ -289,7 +263,7 @@ public void testTextFlowInsertionIndexUsingEmbeddedTextNodes() throws Exception @Test public void testTextFlowInsertionIndexWhenMouseMovedOutsideText() throws Exception { addTextAndEmojis(); - Thread.sleep(500); + Util.waitForIdle(scene); int index = 0; while (index < (HEIGHT - Y_OFFSET)) { @@ -306,7 +280,7 @@ public void testTextFlowInsertionIndexWhenMouseMovedOutsideText() throws Excepti @Test public void testTextFlowInsertionIndexUsingWrappedText() throws Exception { addLongText(); - Thread.sleep(500); + Util.waitForIdle(scene); for (int y = 0; y < 2; y++) { for (int x = 0; x < (WIDTH - X_LEADING_OFFSET); x += 5) { diff --git a/tests/system/src/test/java/test/util/Util.java b/tests/system/src/test/java/test/util/Util.java index e8530be65fc..4c6b66f7b2c 100644 --- a/tests/system/src/test/java/test/util/Util.java +++ b/tests/system/src/test/java/test/util/Util.java @@ -42,6 +42,7 @@ import javafx.application.Platform; import javafx.geometry.Rectangle2D; import javafx.scene.robot.Robot; +import javafx.scene.Scene; import javafx.stage.Screen; import javafx.stage.Stage; @@ -422,4 +423,36 @@ public static void parkCursor(Robot robot) { runAndWait(park); } } + + /** + * Triggers and waits for 10 pulses to complete in the specified scene. + */ + public static void waitForIdle(Scene scene) { + waitForIdle(scene, 10); + } + + /** + * Triggers and waits for specified number of pulses (pulseCount) + * to complete in the specified scene. + */ + public static void waitForIdle(Scene scene, int pulseCount) { + CountDownLatch latch = new CountDownLatch(pulseCount); + Runnable pulseListener = () -> { + latch.countDown(); + Platform.requestNextPulse(); + }; + + runAndWait(() -> { + scene.addPostLayoutPulseListener(pulseListener); + }); + + try { + Platform.requestNextPulse(); + waitForLatch(latch, TIMEOUT, "Timeout waiting for post layout pulse"); + } finally { + runAndWait(() -> { + scene.removePostLayoutPulseListener(pulseListener); + }); + } + } } From ea940bf4ed8d77f16452978765e961a4733eaafd Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 5 Jun 2023 16:41:11 +0530 Subject: [PATCH 8/8] Add out of bound check for insertion index --- .../src/main/java/com/sun/javafx/text/PrismTextLayout.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 5588849c711..3cbbd1ce207 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -454,7 +454,7 @@ public Hit getHitInfo(float x, float y) { leading = (trailing[0] == 0); insertionIndex = charIndex; - if (getText() != null) { + if (getText() != null && insertionIndex < getText().length) { if (!leading) { BreakIterator charIterator = BreakIterator.getCharacterInstance(); charIterator.setText(new String(getText()));