Skip to content

Commit 7b67cc6

Browse files
authored
Fix for #812 caretBounds graphic bug
Remove listeners from selections and carets before clearing in method dispose, otherwise the clear triggers a selection path update that results in an errant selection path appearing at the current cursor position.
1 parent 836902e commit 7b67cc6

File tree

1 file changed

+20
-19
lines changed

1 file changed

+20
-19
lines changed

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

+20-19
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,14 @@
1515
import java.util.function.Supplier;
1616
import java.util.function.UnaryOperator;
1717

18+
import org.fxmisc.richtext.model.Paragraph;
19+
import org.fxmisc.richtext.model.StyledSegment;
20+
import org.reactfx.util.Tuple2;
21+
import org.reactfx.util.Tuples;
22+
import org.reactfx.value.Val;
23+
1824
import javafx.beans.property.ObjectProperty;
1925
import javafx.beans.property.SimpleObjectProperty;
20-
import javafx.beans.value.ChangeListener;
2126
import javafx.collections.FXCollections;
2227
import javafx.collections.MapChangeListener;
2328
import javafx.collections.ObservableMap;
@@ -34,13 +39,7 @@
3439
import javafx.scene.shape.Path;
3540
import javafx.scene.shape.PathElement;
3641
import javafx.scene.shape.StrokeLineCap;
37-
3842
import javafx.scene.shape.StrokeType;
39-
import org.fxmisc.richtext.model.Paragraph;
40-
import org.fxmisc.richtext.model.StyledSegment;
41-
import org.reactfx.util.Tuple2;
42-
import org.reactfx.util.Tuples;
43-
import org.reactfx.value.Val;
4443

4544
/**
4645
* The class responsible for rendering the segments in an paragraph. It also renders additional RichTextFX-specific
@@ -59,8 +58,8 @@ class ParagraphText<PS, SEG, S> extends TextFlowExt {
5958
FXCollections.observableMap(new HashMap<>(1));
6059
public final ObservableMap<Selection<PS, SEG, S>, SelectionPath> selectionsProperty() { return selections; }
6160

62-
private final ChangeListener<IndexRange> selectionRangeListener;
63-
private final ChangeListener<Integer> caretPositionListener;
61+
private final MapChangeListener<? super Selection<PS, SEG, S>, ? super SelectionPath> selectionPathListener;
62+
private final SetChangeListener<? super CaretNode> caretNodeListener;
6463

6564
// FIXME: changing it currently has not effect, because
6665
// Text.impl_selectionFillProperty().set(newFill) doesn't work
@@ -95,47 +94,47 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
9594
Val<Double> leftInset = Val.map(insetsProperty(), Insets::getLeft);
9695
Val<Double> topInset = Val.map(insetsProperty(), Insets::getTop);
9796

98-
selectionRangeListener = (obs, ov, nv) -> requestLayout();
99-
selections.addListener((MapChangeListener.Change<? extends Selection<PS, SEG, S>, ? extends SelectionPath> change) -> {
97+
selectionPathListener = change -> {
10098
if (change.wasRemoved()) {
10199
SelectionPath p = change.getValueRemoved();
102-
p.rangeProperty().removeListener(selectionRangeListener);
100+
p.rangeProperty().removeListener( (obs, ov, nv) -> requestLayout() );
103101
p.layoutXProperty().unbind();
104102
p.layoutYProperty().unbind();
105103

106104
getChildren().remove(p);
107105
}
108106
if (change.wasAdded()) {
109107
SelectionPath p = change.getValueAdded();
110-
p.rangeProperty().addListener(selectionRangeListener);
108+
p.rangeProperty().addListener( (obs, ov, nv) -> requestLayout() );
111109
p.layoutXProperty().bind(leftInset);
112110
p.layoutYProperty().bind(topInset);
113111

114112
getChildren().add(selectionShapeStartIndex, p);
115113
updateSingleSelection(p);
116114
}
117-
});
115+
};
116+
selections.addListener( selectionPathListener );
118117

119-
caretPositionListener = (obs, ov, nv) -> requestLayout();
120-
carets.addListener((SetChangeListener.Change<? extends CaretNode> change) -> {
118+
caretNodeListener = change -> {
121119
if (change.wasRemoved()) {
122120
CaretNode caret = change.getElementRemoved();
123-
caret.columnPositionProperty().removeListener(caretPositionListener);
121+
caret.columnPositionProperty().removeListener( (obs, ov, nv) -> requestLayout() );
124122
caret.layoutXProperty().unbind();
125123
caret.layoutYProperty().unbind();
126124

127125
getChildren().remove(caret);
128126
}
129127
if (change.wasAdded()) {
130128
CaretNode caret = change.getElementAdded();
131-
caret.columnPositionProperty().addListener(caretPositionListener);
129+
caret.columnPositionProperty().addListener( (obs, ov, nv) -> requestLayout() );
132130
caret.layoutXProperty().bind(leftInset);
133131
caret.layoutYProperty().bind(topInset);
134132

135133
getChildren().add(caret);
136134
updateSingleCaret(caret);
137135
}
138-
});
136+
};
137+
carets.addListener( caretNodeListener );
139138

140139
// XXX: see the note at highlightTextFill
141140
// highlightTextFill.addListener(new ChangeListener<Paint>() {
@@ -220,6 +219,8 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
220219

221220
void dispose() {
222221
// this removes listeners (in selections and carets listeners) and avoids memory leaks
222+
selections.removeListener( selectionPathListener );
223+
carets.removeListener( caretNodeListener );
223224
selections.clear();
224225
carets.clear();
225226
}

0 commit comments

Comments
 (0)