Fix bug: Caret/Selection positions should be updated correctly when change occurs at their position

richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/SingleCaretTests.java → richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/caret/BoundsTests.java

@@ -1,27 +1,18 @@
-package org.fxmisc.richtext.api;
+package org.fxmisc.richtext.api.caret;
 
 import javafx.stage.Stage;
 import org.fxmisc.richtext.Caret;
 import org.fxmisc.richtext.InlineCssTextAreaAppTest;
+import org.fxmisc.richtext.TextBuildingUtils;
 import org.junit.Test;
 import org.testfx.util.WaitForAsyncUtils;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-public class SingleCaretTests extends InlineCssTextAreaAppTest {
+public class BoundsTests extends InlineCssTextAreaAppTest {
 
-    private static final String MANY_PARS_OF_TEXT;
-
-    static {
-        int totalLines = 20;
-        StringBuilder sb = new StringBuilder();
-        for (int i = 0; i < totalLines; i++) {
-            sb.append(i).append("\n");
-        }
-        sb.append(totalLines);
-        MANY_PARS_OF_TEXT = sb.toString();
-    }
+    private static final String MANY_PARS_OF_TEXT = TextBuildingUtils.buildLines(20);
 
     @Override
     public void start(Stage stage) throws Exception {

richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/caret/PositionTests.java

@@ -0,0 +1,89 @@
+package org.fxmisc.richtext.api.caret;
+
+import javafx.stage.Stage;
+import org.fxmisc.richtext.CaretNode;
+import org.fxmisc.richtext.InlineCssTextAreaAppTest;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class PositionTests extends InlineCssTextAreaAppTest {
+
+    CaretNode caret;
+    String text = "text";
+
+    @Override
+    public void start(Stage stage) throws Exception {
+        super.start(stage);
+        area.replaceText(text);
+        caret = new CaretNode("extra caret", area);
+        area.addCaret(caret);
+    }
+
+    @Test
+    public void position_is_correct_when_change_occurs_before_position() {
+        interact(() -> {
+            caret.moveToAreaEnd();
+            int pos = caret.getPosition();
+
+            String append = "some";
+
+            // add test
+            area.insertText(0, append);
+            assertEquals(pos + append.length(), caret.getPosition());
+
+            // delete test
+            area.deleteText(0, append.length());
+            assertEquals(pos, caret.getPosition());
+        });
+    }
+
+    @Test
+    public void position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
+        interact(() -> {
+            caret.moveTo(text.length() - 1);
+
+            area.appendText("append");
+            area.deleteText(0, text.length());
+            assertEquals(0, caret.getPosition());
+        });
+    }
+
+    @Test
+    public void position_is_correct_when_change_occurs_at_position() {
+        interact(() -> {
+            caret.moveToAreaEnd();
+            int pos = caret.getPosition();
+
+            String append = "some";
+            // add test
+            area.appendText(append);
+            assertEquals(pos + append.length(), caret.getPosition());
+
+            // reset
+            caret.moveTo(pos);
+
+            // delete test
+            area.deleteText(pos, area.getLength());
+            assertEquals(pos, caret.getPosition());
+        });
+    }
+
+    @Test
+    public void position_is_correct_when_change_occurs_after_position() {
+        interact(() -> {
+            caret.moveTo(0);
+
+            // add test
+            String append = "some";
+            area.appendText(append);
+            assertEquals(0, caret.getPosition());
+
+            // delete test
+            int length = area.getLength();
+            area.deleteText(length - append.length(), length);
+            assertEquals(0, caret.getPosition());
+        });
+    }
+
+}

richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/selection/PositionTests.java

@@ -0,0 +1,170 @@
+package org.fxmisc.richtext.api.selection;
+
+import javafx.stage.Stage;
+import org.fxmisc.richtext.InlineCssTextAreaAppTest;
+import org.fxmisc.richtext.Selection;
+import org.fxmisc.richtext.SelectionImpl;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class PositionTests extends InlineCssTextAreaAppTest {
+
+    private String leftText = "left";
+    private String rightText = "right";
+    private String fullText = leftText + rightText;
+
+    private Selection<String, String, String> selection;
+
+    @Override
+    public void start(Stage stage) throws Exception {
+        super.start(stage);
+        area.replaceText(fullText);
+        selection = new SelectionImpl<>("extra selection", area);
+        area.addSelection(selection);
+    }
+
+    private void selectLeft() {
+        selection.selectRange(0, leftText.length());
+    }
+
+    private void selectRight() {
+        selection.selectRange(leftText.length(), area.getLength());
+    }
+
+    @Test
+    public void start_position_is_correct_when_change_occurs_before_position() {
+        interact(() -> {
+            selectLeft();
+            int pos = selection.getStartPosition();
+
+            String append = "some";
+
+            // add test
+            area.insertText(0, append);
+            assertEquals(pos + append.length(), selection.getStartPosition());
+
+            // delete test
+            area.deleteText(0, append.length());
+            assertEquals(pos, selection.getStartPosition());
+        });
+    }
+
+    @Test
+    public void start_position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
+        interact(() -> {
+            selection.selectRange(2, leftText.length() + 1);
+
+            area.deleteText(0, leftText.length());
+            assertEquals(0, selection.getStartPosition());
+        });
+    }
+
+    @Test
+    public void start_position_is_correct_when_change_occurs_at_position() {
+        interact(() -> {
+            selectRight();
+            int pos = selection.getStartPosition();
+
+            String append = "some";
+            // add test
+            area.insertText(leftText.length(), append);
+            assertEquals(pos + append.length(), selection.getStartPosition());
+
+            // reset
+            selection.updateStartTo(pos);
+
+            // delete test
+            area.deleteText(pos, append.length());
+            assertEquals(pos, selection.getStartPosition());
+        });
+    }
+
+    @Test
+    public void start_position_is_correct_when_change_occurs_after_position() {
+        interact(() -> {
+            selectLeft();
+
+            // add test
+            String append = "some";
+            area.appendText(append);
+            assertEquals(0, selection.getStartPosition());
+
+            // delete test
+            int length = area.getLength();
+            area.deleteText(length - append.length(), length);
+            assertEquals(0, selection.getStartPosition());
+        });
+    }
+
+    @Test
+    public void end_position_is_correct_when_change_occurs_before_position() {
+        interact(() -> {
+            selectRight();
+            int pos = selection.getEndPosition();
+
+            String append = "some";
+
+            // add test
+            area.insertText(0, append);
+            assertEquals(pos + append.length(), selection.getEndPosition());
+
+            // delete test
+            area.deleteText(0, append.length());
+            assertEquals(pos, selection.getEndPosition());
+        });
+    }
+
+    @Test
+    public void end_position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
+        interact(() -> {
+            selection.selectRange(leftText.length() - 1, area.getLength());
+
+            area.deleteText(leftText.length(), area.getLength());
+            assertEquals(leftText.length(), selection.getEndPosition());
+        });
+    }
+
+    @Test
+    public void end_position_is_correct_when_change_occurs_at_position() {
+        interact(() -> {
+            selectLeft();
+            int pos = selection.getEndPosition();
+
+            String append = "some";
+            // add test
+            area.insertText(leftText.length(), append);
+            assertEquals(pos, selection.getEndPosition());
+
+            // delete test
+            area.deleteText(pos, area.getLength());
+            assertEquals(pos, selection.getEndPosition());
+        });
+    }
+
+    @Test
+    public void end_position_is_correct_when_change_occurs_after_position() {
+        interact(() -> {
+            selectLeft();
+
+            // add test
+            String append = "some";
+            area.appendText(append);
+            assertEquals(leftText.length(), selection.getEndPosition());
+
+            // delete test
+            int length = area.getLength();
+            area.deleteText(length - append.length(), length);
+            assertEquals(leftText.length(), selection.getEndPosition());
+        });
+    }
+
+    public void deletion_which_includes_selection_and_which_occurs_at_end_of_area_moves_selection_to_new_area_end() {
+        interact(() -> {
+           selection.selectRange(area.getLength(), area.getLength());
+           area.deleteText(leftText.length(), area.getLength());
+           assertEquals(area.getLength(), selection.getStartPosition());
+           assertEquals(area.getLength(), selection.getEndPosition());
+        });
+    }
+}

richtextfx/src/main/java/org/fxmisc/richtext/CaretNode.java

@@ -165,9 +165,23 @@ public CaretNode(String name, GenericStyledArea<?, ?, ?> area, SuspendableNo dep
                     // in case of a replacement: "hello there" -> "hi."
                     int endOfChange = indexOfChange + Math.abs(netLength);
 
-                    if (indexOfChange < finalPosition) {
-                        // if caret is within the changed content, move it to indexOfChange
-                        // otherwise offset it by netLength
+                    /*
+                        "->" means add (positive) netLength to position
+                        "<-" means add (negative) netLength to position
+                        "x" means don't update position
+
+                        "+c" means caret was included in the deleted portion of content
+                        "-c" means caret was not included in the deleted portion of content
+                        Before/At/After means indexOfChange "<" / "==" / ">" position
+
+                               |   Before +c   | Before -c | At | After
+                        -------+---------------+-----------+----+------
+                        Add    |      N/A      |    ->     | -> | x
+                        Delete | indexOfChange |    <-     | x  | x
+                     */
+                    if (indexOfChange == finalPosition && netLength > 0) {
+                        finalPosition = finalPosition + netLength;
+                    } else if (indexOfChange < finalPosition) {
                         finalPosition = finalPosition < endOfChange
                                         ? indexOfChange
                                         : finalPosition + netLength;

richtextfx/src/main/java/org/fxmisc/richtext/SelectionImpl.java

@@ -232,10 +232,25 @@ public SelectionImpl(String name, GenericStyledArea<PS, SEG, S> area, int startP
 
                     if (getLength() != 0) {
 
-                        // if start/end is within the changed content, move it to indexOfChange
-                        // otherwise, offset it by netLength
-                        // Note: if both are moved to indexOfChange, selection is empty.
-                        if (indexOfChange < finalStart) {
+                        /*
+                            "->" means add (positive) netLength to position
+                            "<-" means add (negative) netLength to position
+                            "x" means don't update position
+
+                            "start / end" means what should be done in each case for each anchor if they differ
+
+                            "+a" means one of the anchors was included in the deleted portion of content
+                            "-a" means one of the anchors was not included in the deleted portion of content
+                            Before/At/After means indexOfChange "<" / "==" / ">" position
+
+                                   |   Before +a   | Before -a |   At   | After
+                            -------+---------------+-----------+--------+------
+                            Add    |      N/A      |    ->     | -> / x | x
+                            Delete | indexOfChange |    <-     |    x   | x
+                         */
+                        if (indexOfChange == finalStart && netLength > 0) {
+                            finalStart = finalStart + netLength;
+                        } else if (indexOfChange < finalStart) {
                             finalStart = finalStart < endOfChange
                                     ? indexOfChange
                                     : finalStart + netLength;