tobiasdiez / EasyBind

Custom JavaFX bindings made easy with lambdas.
BSD 2-Clause "Simplified" License
31 stars 6 forks source link

EasyBind.map() bug when binding to Pane children and using remove() #82

Open credmond opened 4 months ago

credmond commented 4 months ago

A "Children: duplicate children added: parent" exception occurs whenever remove(x) is called on a source observable list.

Very easy to reproduce:

public class EasyBindMapBug extends Application {

    private final ObservableList<String> personList = FXCollections.observableArrayList("a", "b", "c");

    @Override
    public void start(Stage primaryStage) {
        final ObservableList<Text> textList = EasyBind.map(personList, str -> {
                    System.out.println("XXX: " + str);

                    return new Text(str) {
                // If you override hashCode/equals to prevent duplications, the problem does NOT occur
                //  @Override
                //  public int hashCode() {
                //      return str.hashCode();
                //  }
                //
                //  @Override
                //  public boolean equals(Object other) {
                //      return ((Text) other).getText().equals(this.getText());
                //  }
                    };
                }
        );

        // No problem for controls, this will work just fine
        // ListView<Text> node = new ListView<>();
        // Bindings.bindContent(node.getItems(), textList);

        final VBox node = new VBox();
        Bindings.bindContent(node.getChildren(), textList);

        primaryStage.setScene(new Scene(node, 300, 200));
        primaryStage.show();

        // This will throw a "Children: duplicate children added: parent" error, all the time
        personList.remove(1); // or 0, or 2, etc...
    }

    public static void main(String[] args) {
        launch(args);
    }
}

Stacktrace:

Exception in thread "JavaFX Application Thread" java.lang.IllegalArgumentException: Children: duplicate children added: parent = VBox@62f1ef9f[styleClass=root]
    at javafx.graphics@21.0.3/javafx.scene.Parent$3.onProposedChange(Parent.java:562)
    at javafx.base@21.0.3/com.sun.javafx.collections.VetoableListDecorator$VetoableSubListDecorator.clear(VetoableListDecorator.java:544)
    at com.tobiasdiez.easybind@2.2.1-SNAPSHOT/com.tobiasdiez.easybind.ListContentBinding.onChanged(ListContentBinding.java:35)
    at javafx.base@21.0.3/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
    at javafx.base@21.0.3/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
    at javafx.base@21.0.3/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
    at com.tobiasdiez.easybind@2.2.1-SNAPSHOT/com.tobiasdiez.easybind.MappedList.sourceChanged(MappedList.java:41)
    at javafx.base@21.0.3/javafx.collections.transformation.TransformationList.lambda$getListener$0(TransformationList.java:105)
    at javafx.base@21.0.3/javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
    at javafx.base@21.0.3/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
    at javafx.base@21.0.3/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
    at javafx.base@21.0.3/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
    at javafx.base@21.0.3/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
    at javafx.base@21.0.3/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
    at javafx.base@21.0.3/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
    at javafx.base@21.0.3/javafx.collections.ModifiableObservableListBase.remove(ModifiableObservableListBase.java:228)
    at com.certak.xxx/com.certak.xxx.gui.aaa.EasyBindMapBug.start(EasyBindMapBug.java:48)
    at javafx.graphics@21.0.3/com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$9(LauncherImpl.java:839)
    at javafx.graphics@21.0.3/com.sun.javafx.application.PlatformImpl.lambda$runAndWait$12(PlatformImpl.java:483)
    at javafx.graphics@21.0.3/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
    at javafx.graphics@21.0.3/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
    at javafx.graphics@21.0.3/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
    at javafx.graphics@21.0.3/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
    at javafx.graphics@21.0.3/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:185)
    at java.base/java.lang.Thread.run(Thread.java:833)

As per the comments, binding to a control's items, for example, is fine. Also, mapping to something that has sensible hashCode/equals may also fix the problem. It's still a bug though, and could do with a documentation fix.

Using mapBacked() would be okay too as the items are only generated once per change to source.