Fix ChartPlugin scene listener is not removed (#676)

Change ChartPlugin to remove all listeners once a plugin is removed from it's chart and
add a unit-test to verify that this case is correctly handled.

Author: amischler

chartfx-chart/src/main/java/io/fair_acc/chartfx/plugins/ChartPlugin.java

@@ -1,18 +1,22 @@
 package io.fair_acc.chartfx.plugins;
 
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.property.SimpleObjectProperty;
+import javafx.beans.value.ChangeListener;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.event.EventHandler;
 import javafx.event.EventType;
 import javafx.geometry.Point2D;
 import javafx.scene.Node;
+import javafx.scene.Scene;
 import javafx.scene.canvas.Canvas;
 import javafx.scene.input.InputEvent;
 import javafx.scene.input.MouseEvent;
@@ -45,6 +49,7 @@ public abstract class ChartPlugin implements Measurable.EmptyDefault {
     private static final Logger LOGGER = LoggerFactory.getLogger(ChartPlugin.class);
     private final ObservableList<Node> chartChildren = FXCollections.observableArrayList();
     private final List<Pair<EventType<? extends InputEvent>, EventHandler<? extends InputEvent>>> mouseEventHandlers = new LinkedList<>();
+    private final Map<Node, ChangeListener<? super Scene>> sceneChangeListeners = new HashMap<>();
 
     /**
      * The associated {@link Chart}. Initialised when the plug-in is added to the Chart, set to {@code null} when
@@ -90,19 +95,29 @@ private <T extends InputEvent> void addEventHandlers(final Node node) {
             final EventType<T> type = (EventType<T>) pair.getKey();
             final EventHandler<T> handler = (EventHandler<T>) pair.getValue();
             node.addEventHandler(type, handler);
-            node.sceneProperty().addListener((ch, o, n) -> {
-                if (o == n) {
-                    return;
-                }
-                if (o != null) {
+        }
+        ChangeListener<? super Scene> sceneListener = (ch, o, n) -> {
+            if (o == n) {
+                return;
+            }
+            if (o != null) {
+                for (final Pair<EventType<? extends InputEvent>, EventHandler<? extends InputEvent>> pair : mouseEventHandlers) {
+                    final EventType<T> type = (EventType<T>) pair.getKey();
+                    final EventHandler<T> handler = (EventHandler<T>) pair.getValue();
                     o.removeEventHandler(type, handler);
                 }
+            }
 
-                if (n != null) {
+            if (n != null) {
+                for (final Pair<EventType<? extends InputEvent>, EventHandler<? extends InputEvent>> pair : mouseEventHandlers) {
+                    final EventType<T> type = (EventType<T>) pair.getKey();
+                    final EventHandler<T> handler = (EventHandler<T>) pair.getValue();
                     n.addEventHandler(type, handler);
                 }
-            });
-        }
+            }
+        };
+        sceneChangeListeners.put(node, sceneListener);
+        node.sceneProperty().addListener(sceneListener);
     }
 
     /**
@@ -230,9 +245,11 @@ private <T extends InputEvent> void removeEventHandlers(final Node node) {
             final EventHandler<T> handler = (EventHandler<T>) pair.getValue();
             node.removeEventHandler(type, handler);
             if (node.getScene() != null) {
-                node.getScene().removeEventFilter(type, handler);
+                node.getScene().removeEventHandler(type, handler);
             }
         }
+        node.sceneProperty().removeListener(sceneChangeListeners.get(node));
+        sceneChangeListeners.put(node, null);
     }
 
     /**

chartfx-chart/src/test/java/io/fair_acc/chartfx/plugins/ChartPluginTest.java

@@ -0,0 +1,71 @@
+package io.fair_acc.chartfx.plugins;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import javafx.scene.Scene;
+import javafx.scene.control.Label;
+import javafx.scene.input.MouseButton;
+import javafx.scene.input.MouseEvent;
+import javafx.scene.layout.BorderPane;
+import javafx.stage.Stage;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.testfx.api.FxRobot;
+import org.testfx.framework.junit5.ApplicationExtension;
+import org.testfx.framework.junit5.Start;
+
+import io.fair_acc.chartfx.XYChart;
+import io.fair_acc.chartfx.axes.spi.DefaultNumericAxis;
+import io.fair_acc.chartfx.utils.FXUtils;
+
+@ExtendWith(ApplicationExtension.class)
+class ChartPluginTest {
+    static class TestChartPlugin extends ChartPlugin {
+        public TestChartPlugin() {
+            registerInputEventHandler(MouseEvent.MOUSE_CLICKED, this::handle);
+        }
+
+        boolean clicked = false;
+
+        private void handle(MouseEvent mouseEvent) {
+            clicked = true;
+        }
+    }
+
+    private XYChart chart;
+    private Label label;
+    private BorderPane root;
+
+    @Start
+    void start(Stage stage) {
+        chart = new XYChart(new DefaultNumericAxis(), new DefaultNumericAxis());
+        root = new BorderPane(chart);
+        label = new Label("Click");
+        root.setBottom(label);
+        Scene scene = new Scene(root, 500, 400);
+        stage.setScene(scene);
+        stage.show();
+    }
+
+    @Test
+    void testSceneListenerIsRemoved(FxRobot robot) {
+        assertNotNull(chart);
+        assertEquals(0, chart.getPlugins().size());
+        final TestChartPlugin testChartPlugin = new TestChartPlugin();
+        assertDoesNotThrow(() -> FXUtils.runAndWait(() -> chart.getPlugins().add(testChartPlugin)));
+        assertEquals(1, chart.getPlugins().size());
+        robot.moveTo(chart).clickOn(MouseButton.PRIMARY).interrupt();
+        assertTrue(testChartPlugin.clicked);
+        assertDoesNotThrow(() -> FXUtils.runAndWait(() -> chart.getPlugins().remove(testChartPlugin)));
+        assertEquals(0, chart.getPlugins().size());
+        testChartPlugin.clicked = false;
+        assertDoesNotThrow(() -> FXUtils.runAndWait(() -> {
+            root.setCenter(null);
+            root.setCenter(chart);
+        }));
+        robot.moveTo(label).clickOn(MouseButton.PRIMARY).interrupt();
+        assertFalse(testChartPlugin.clicked);
+    }
+}