Skip to content

Commit 9564b94

Browse files
JordanMartinezJordanMartinez
authored
Merge pull request #766 from JordanMartinez/fixPosUpdate
Fix bug: Caret/Selection positions should be updated correctly when change occurs at their position
2 parents b108e26 + dae3cbd commit 9564b94

5 files changed

Lines changed: 299 additions & 20 deletions

File tree

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,18 @@
1-
package org.fxmisc.richtext.api;
1+
package org.fxmisc.richtext.api.caret;
22

33
import javafx.stage.Stage;
44
import org.fxmisc.richtext.Caret;
55
import org.fxmisc.richtext.InlineCssTextAreaAppTest;
6+
import org.fxmisc.richtext.TextBuildingUtils;
67
import org.junit.Test;
78
import org.testfx.util.WaitForAsyncUtils;
89

910
import static org.junit.Assert.assertFalse;
1011
import static org.junit.Assert.assertTrue;
1112

12-
public class SingleCaretTests extends InlineCssTextAreaAppTest {
13+
public class BoundsTests extends InlineCssTextAreaAppTest {
1314

14-
private static final String MANY_PARS_OF_TEXT;
15-
16-
static {
17-
int totalLines = 20;
18-
StringBuilder sb = new StringBuilder();
19-
for (int i = 0; i < totalLines; i++) {
20-
sb.append(i).append("\n");
21-
}
22-
sb.append(totalLines);
23-
MANY_PARS_OF_TEXT = sb.toString();
24-
}
15+
private static final String MANY_PARS_OF_TEXT = TextBuildingUtils.buildLines(20);
2516

2617
@Override
2718
public void start(Stage stage) throws Exception {

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package org.fxmisc.richtext.api.caret;
2+
3+
import javafx.stage.Stage;
4+
import org.fxmisc.richtext.CaretNode;
5+
import org.fxmisc.richtext.InlineCssTextAreaAppTest;
6+
import org.junit.Test;
7+
8+
import static org.junit.Assert.assertEquals;
9+
10+
public class PositionTests extends InlineCssTextAreaAppTest {
11+
12+
CaretNode caret;
13+
String text = "text";
14+
15+
@Override
16+
public void start(Stage stage) throws Exception {
17+
super.start(stage);
18+
area.replaceText(text);
19+
caret = new CaretNode("extra caret", area);
20+
area.addCaret(caret);
21+
}
22+
23+
@Test
24+
public void position_is_correct_when_change_occurs_before_position() {
25+
interact(() -> {
26+
caret.moveToAreaEnd();
27+
int pos = caret.getPosition();
28+
29+
String append = "some";
30+
31+
// add test
32+
area.insertText(0, append);
33+
assertEquals(pos + append.length(), caret.getPosition());
34+
35+
// delete test
36+
area.deleteText(0, append.length());
37+
assertEquals(pos, caret.getPosition());
38+
});
39+
}
40+
41+
@Test
42+
public void position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
43+
interact(() -> {
44+
caret.moveTo(text.length() - 1);
45+
46+
area.appendText("append");
47+
area.deleteText(0, text.length());
48+
assertEquals(0, caret.getPosition());
49+
});
50+
}
51+
52+
@Test
53+
public void position_is_correct_when_change_occurs_at_position() {
54+
interact(() -> {
55+
caret.moveToAreaEnd();
56+
int pos = caret.getPosition();
57+
58+
String append = "some";
59+
// add test
60+
area.appendText(append);
61+
assertEquals(pos + append.length(), caret.getPosition());
62+
63+
// reset
64+
caret.moveTo(pos);
65+
66+
// delete test
67+
area.deleteText(pos, area.getLength());
68+
assertEquals(pos, caret.getPosition());
69+
});
70+
}
71+
72+
@Test
73+
public void position_is_correct_when_change_occurs_after_position() {
74+
interact(() -> {
75+
caret.moveTo(0);
76+
77+
// add test
78+
String append = "some";
79+
area.appendText(append);
80+
assertEquals(0, caret.getPosition());
81+
82+
// delete test
83+
int length = area.getLength();
84+
area.deleteText(length - append.length(), length);
85+
assertEquals(0, caret.getPosition());
86+
});
87+
}
88+
89+
}

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

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package org.fxmisc.richtext.api.selection;
2+
3+
import javafx.stage.Stage;
4+
import org.fxmisc.richtext.InlineCssTextAreaAppTest;
5+
import org.fxmisc.richtext.Selection;
6+
import org.fxmisc.richtext.SelectionImpl;
7+
import org.junit.Test;
8+
9+
import static org.junit.Assert.assertEquals;
10+
11+
public class PositionTests extends InlineCssTextAreaAppTest {
12+
13+
private String leftText = "left";
14+
private String rightText = "right";
15+
private String fullText = leftText + rightText;
16+
17+
private Selection<String, String, String> selection;
18+
19+
@Override
20+
public void start(Stage stage) throws Exception {
21+
super.start(stage);
22+
area.replaceText(fullText);
23+
selection = new SelectionImpl<>("extra selection", area);
24+
area.addSelection(selection);
25+
}
26+
27+
private void selectLeft() {
28+
selection.selectRange(0, leftText.length());
29+
}
30+
31+
private void selectRight() {
32+
selection.selectRange(leftText.length(), area.getLength());
33+
}
34+
35+
@Test
36+
public void start_position_is_correct_when_change_occurs_before_position() {
37+
interact(() -> {
38+
selectLeft();
39+
int pos = selection.getStartPosition();
40+
41+
String append = "some";
42+
43+
// add test
44+
area.insertText(0, append);
45+
assertEquals(pos + append.length(), selection.getStartPosition());
46+
47+
// delete test
48+
area.deleteText(0, append.length());
49+
assertEquals(pos, selection.getStartPosition());
50+
});
51+
}
52+
53+
@Test
54+
public void start_position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
55+
interact(() -> {
56+
selection.selectRange(2, leftText.length() + 1);
57+
58+
area.deleteText(0, leftText.length());
59+
assertEquals(0, selection.getStartPosition());
60+
});
61+
}
62+
63+
@Test
64+
public void start_position_is_correct_when_change_occurs_at_position() {
65+
interact(() -> {
66+
selectRight();
67+
int pos = selection.getStartPosition();
68+
69+
String append = "some";
70+
// add test
71+
area.insertText(leftText.length(), append);
72+
assertEquals(pos + append.length(), selection.getStartPosition());
73+
74+
// reset
75+
selection.updateStartTo(pos);
76+
77+
// delete test
78+
area.deleteText(pos, append.length());
79+
assertEquals(pos, selection.getStartPosition());
80+
});
81+
}
82+
83+
@Test
84+
public void start_position_is_correct_when_change_occurs_after_position() {
85+
interact(() -> {
86+
selectLeft();
87+
88+
// add test
89+
String append = "some";
90+
area.appendText(append);
91+
assertEquals(0, selection.getStartPosition());
92+
93+
// delete test
94+
int length = area.getLength();
95+
area.deleteText(length - append.length(), length);
96+
assertEquals(0, selection.getStartPosition());
97+
});
98+
}
99+
100+
@Test
101+
public void end_position_is_correct_when_change_occurs_before_position() {
102+
interact(() -> {
103+
selectRight();
104+
int pos = selection.getEndPosition();
105+
106+
String append = "some";
107+
108+
// add test
109+
area.insertText(0, append);
110+
assertEquals(pos + append.length(), selection.getEndPosition());
111+
112+
// delete test
113+
area.deleteText(0, append.length());
114+
assertEquals(pos, selection.getEndPosition());
115+
});
116+
}
117+
118+
@Test
119+
public void end_position_is_correct_when_change_occurs_before_position_and_deletes_carets_position() {
120+
interact(() -> {
121+
selection.selectRange(leftText.length() - 1, area.getLength());
122+
123+
area.deleteText(leftText.length(), area.getLength());
124+
assertEquals(leftText.length(), selection.getEndPosition());
125+
});
126+
}
127+
128+
@Test
129+
public void end_position_is_correct_when_change_occurs_at_position() {
130+
interact(() -> {
131+
selectLeft();
132+
int pos = selection.getEndPosition();
133+
134+
String append = "some";
135+
// add test
136+
area.insertText(leftText.length(), append);
137+
assertEquals(pos, selection.getEndPosition());
138+
139+
// delete test
140+
area.deleteText(pos, area.getLength());
141+
assertEquals(pos, selection.getEndPosition());
142+
});
143+
}
144+
145+
@Test
146+
public void end_position_is_correct_when_change_occurs_after_position() {
147+
interact(() -> {
148+
selectLeft();
149+
150+
// add test
151+
String append = "some";
152+
area.appendText(append);
153+
assertEquals(leftText.length(), selection.getEndPosition());
154+
155+
// delete test
156+
int length = area.getLength();
157+
area.deleteText(length - append.length(), length);
158+
assertEquals(leftText.length(), selection.getEndPosition());
159+
});
160+
}
161+
162+
public void deletion_which_includes_selection_and_which_occurs_at_end_of_area_moves_selection_to_new_area_end() {
163+
interact(() -> {
164+
selection.selectRange(area.getLength(), area.getLength());
165+
area.deleteText(leftText.length(), area.getLength());
166+
assertEquals(area.getLength(), selection.getStartPosition());
167+
assertEquals(area.getLength(), selection.getEndPosition());
168+
});
169+
}
170+
}

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,23 @@ public CaretNode(String name, GenericStyledArea<?, ?, ?> area, SuspendableNo dep
165165
// in case of a replacement: "hello there" -> "hi."
166166
int endOfChange = indexOfChange + Math.abs(netLength);
167167

168-
if (indexOfChange < finalPosition) {
169-
// if caret is within the changed content, move it to indexOfChange
170-
// otherwise offset it by netLength
168+
/*
169+
"->" means add (positive) netLength to position
170+
"<-" means add (negative) netLength to position
171+
"x" means don't update position
172+
173+
"+c" means caret was included in the deleted portion of content
174+
"-c" means caret was not included in the deleted portion of content
175+
Before/At/After means indexOfChange "<" / "==" / ">" position
176+
177+
| Before +c | Before -c | At | After
178+
-------+---------------+-----------+----+------
179+
Add | N/A | -> | -> | x
180+
Delete | indexOfChange | <- | x | x
181+
*/
182+
if (indexOfChange == finalPosition && netLength > 0) {
183+
finalPosition = finalPosition + netLength;
184+
} else if (indexOfChange < finalPosition) {
171185
finalPosition = finalPosition < endOfChange
172186
? indexOfChange
173187
: finalPosition + netLength;

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,25 @@ public SelectionImpl(String name, GenericStyledArea<PS, SEG, S> area, int startP
232232

233233
if (getLength() != 0) {
234234

235-
// if start/end is within the changed content, move it to indexOfChange
236-
// otherwise, offset it by netLength
237-
// Note: if both are moved to indexOfChange, selection is empty.
238-
if (indexOfChange < finalStart) {
235+
/*
236+
"->" means add (positive) netLength to position
237+
"<-" means add (negative) netLength to position
238+
"x" means don't update position
239+
240+
"start / end" means what should be done in each case for each anchor if they differ
241+
242+
"+a" means one of the anchors was included in the deleted portion of content
243+
"-a" means one of the anchors was not included in the deleted portion of content
244+
Before/At/After means indexOfChange "<" / "==" / ">" position
245+
246+
| Before +a | Before -a | At | After
247+
-------+---------------+-----------+--------+------
248+
Add | N/A | -> | -> / x | x
249+
Delete | indexOfChange | <- | x | x
250+
*/
251+
if (indexOfChange == finalStart && netLength > 0) {
252+
finalStart = finalStart + netLength;
253+
} else if (indexOfChange < finalStart) {
239254
finalStart = finalStart < endOfChange
240255
? indexOfChange
241256
: finalStart + netLength;

0 commit comments

Comments
 (0)
⚡