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.
FXMisc · May 14, 2019 · 7b67cc6 · 7b67cc6
1 parent 836902e
commit 7b67cc6
Showing 1 changed file with 20 additions and 19 deletions.
diff --git a/richtextfx/src/main/java/org/fxmisc/richtext/ParagraphText.java b/richtextfx/src/main/java/org/fxmisc/richtext/ParagraphText.java
@@ -15,9 +15,14 @@
 import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
 
+import org.fxmisc.richtext.model.Paragraph;
+import org.fxmisc.richtext.model.StyledSegment;
+import org.reactfx.util.Tuple2;
+import org.reactfx.util.Tuples;
+import org.reactfx.value.Val;
+
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
-import javafx.beans.value.ChangeListener;
 import javafx.collections.FXCollections;
 import javafx.collections.MapChangeListener;
 import javafx.collections.ObservableMap;
@@ -34,13 +39,7 @@
 import javafx.scene.shape.Path;
 import javafx.scene.shape.PathElement;
 import javafx.scene.shape.StrokeLineCap;
-
 import javafx.scene.shape.StrokeType;
-import org.fxmisc.richtext.model.Paragraph;
-import org.fxmisc.richtext.model.StyledSegment;
-import org.reactfx.util.Tuple2;
-import org.reactfx.util.Tuples;
-import org.reactfx.value.Val;
 
 /**
  * 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 {
             FXCollections.observableMap(new HashMap<>(1));
     public final ObservableMap<Selection<PS, SEG, S>, SelectionPath> selectionsProperty() { return selections; }
 
-    private final ChangeListener<IndexRange> selectionRangeListener;
-    private final ChangeListener<Integer> caretPositionListener;
+    private final MapChangeListener<? super Selection<PS, SEG, S>, ? super SelectionPath> selectionPathListener;
+    private final SetChangeListener<? super CaretNode> caretNodeListener;
 
     // FIXME: changing it currently has not effect, because
     // Text.impl_selectionFillProperty().set(newFill) doesn't work
@@ -95,47 +94,47 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
         Val<Double> leftInset = Val.map(insetsProperty(), Insets::getLeft);
         Val<Double> topInset = Val.map(insetsProperty(), Insets::getTop);
 
-        selectionRangeListener = (obs, ov, nv) -> requestLayout();
-        selections.addListener((MapChangeListener.Change<? extends Selection<PS, SEG, S>, ? extends SelectionPath> change) -> {
+        selectionPathListener = change -> {
             if (change.wasRemoved()) {
                 SelectionPath p = change.getValueRemoved();
-                p.rangeProperty().removeListener(selectionRangeListener);
+                p.rangeProperty().removeListener( (obs, ov, nv) -> requestLayout() );
                 p.layoutXProperty().unbind();
                 p.layoutYProperty().unbind();
 
                 getChildren().remove(p);
             }
             if (change.wasAdded()) {
                 SelectionPath p = change.getValueAdded();
-                p.rangeProperty().addListener(selectionRangeListener);
+                p.rangeProperty().addListener( (obs, ov, nv) -> requestLayout() );
                 p.layoutXProperty().bind(leftInset);
                 p.layoutYProperty().bind(topInset);
 
                 getChildren().add(selectionShapeStartIndex, p);
                 updateSingleSelection(p);
             }
-        });
+        };
+        selections.addListener( selectionPathListener );
 
-        caretPositionListener = (obs, ov, nv) -> requestLayout();
-        carets.addListener((SetChangeListener.Change<? extends CaretNode> change) -> {
+        caretNodeListener = change -> {
             if (change.wasRemoved()) {
                 CaretNode caret = change.getElementRemoved();
-                caret.columnPositionProperty().removeListener(caretPositionListener);
+                caret.columnPositionProperty().removeListener( (obs, ov, nv) -> requestLayout() );
                 caret.layoutXProperty().unbind();
                 caret.layoutYProperty().unbind();
 
                 getChildren().remove(caret);
             }
             if (change.wasAdded()) {
                 CaretNode caret = change.getElementAdded();
-                caret.columnPositionProperty().addListener(caretPositionListener);
+                caret.columnPositionProperty().addListener( (obs, ov, nv) -> requestLayout() );
                 caret.layoutXProperty().bind(leftInset);
                 caret.layoutYProperty().bind(topInset);
 
                 getChildren().add(caret);
                 updateSingleCaret(caret);
             }
-        });
+        };
+        carets.addListener( caretNodeListener );
 
         // XXX: see the note at highlightTextFill
 //        highlightTextFill.addListener(new ChangeListener<Paint>() {
@@ -220,6 +219,8 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
 
     void dispose() {
         // this removes listeners (in selections and carets listeners) and avoids memory leaks
+    	selections.removeListener( selectionPathListener );
+    	carets.removeListener( caretNodeListener );
         selections.clear();
         carets.clear();
     }