user5182503
user5182503

Reputation:

Double tab change event on tab closing in JavaFX

I have the following program:

public class MainClass extends Application {

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

    @Override
    public void start(Stage primaryStage) throws Exception {
        TabPane tabPane = new TabPane();
        tabPane.getSelectionModel().selectedItemProperty().addListener((ov, oldTab, newTab) -> {
            System.out.println("Tab change: " + oldTab + "/" + newTab);
        });
        Tab tab = new Tab("Test tab");
        tab.setOnCloseRequest((event) -> {
            System.out.println("Removing tab");
            event.consume();
            //I need to remove tab manually
            tabPane.getTabs().remove(tab);
        });
        System.out.println("Adding tab");
        tabPane.getTabs().add(tab);
        Group root = new Group(tabPane);
        Scene scene = new Scene(root);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

}

When I run it and click close icon on Tab I have the following output of the program:

Adding tab
Tab change: null/javafx.scene.control.Tab@70b1aa69
Removing tab
Tab change: javafx.scene.control.Tab@70b1aa69/null
Tab change: null/javafx.scene.control.Tab@70b1aa69

As you see I get two Tab change events when I closing tab but I need only one. How to fix it?

Upvotes: 2

Views: 714

Answers (2)

user5182503
user5182503

Reputation:

It seems to be a bug so I opened a bug issue http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8189424 (https://bugs.openjdk.java.net/browse/JDK-8189424) and accept this answer (as soon as SO lets me do it).

Upvotes: 1

kleopatra
kleopatra

Reputation: 51525

Interesting bug - was puzzled as to why/how the removed tab can still be the selected tab even though no longer in the tabs list.

First question was, where exactly the selection happens: that's done in the mousePressedHandler installed by the TabHeaderSkin

setOnMousePressed(new EventHandler<MouseEvent>() {
    @Override public void handle(MouseEvent me) {
        if (getTab().isDisable()) {
            return;
        }
        if (me.getButton().equals(MouseButton.MIDDLE)) {
            if (showCloseButton()) {
                Tab tab = getTab();
                if (behavior.canCloseTab(tab)) {
                    removeListeners(tab);
                    behavior.closeTab(tab);
                }
            }
        } else if (me.getButton().equals(MouseButton.PRIMARY)) {
            behavior.selectTab(getTab());
        }
    }
});

But then: how comes that this handler is still active after removal of the tab (and hopefully its visuals as well)? The cleanup of the visual parts is the task of TabPaneSkin, it listens to the tabs list and removes the TabHeaderSkin (aka: the component that shows the tab above its content). But the cleanup is not immediately complete for two reasons:

  • fade-out animation keeps the header alive until the animation is ready, that's fine
  • header's internal cleanup (messaged via header.removeListeners) is incomplete, as it removes children and listeners, but fails to remove the mouseHandler - and that's the bug.

Code from TabHeaderSkin:

private void removeListeners(Tab tab) {
    listener.dispose();
    inner.getChildren().clear();
    getChildren().clear();
    // following line is missing:
    setOnMousePressed(null)   
}

A way to hack around is to register our own listener on tabs, and force the handler to null on removal. Note: the listener must be notified after core did its job, so either install in a custom skin or after the tabPane's skin has been set.

To illustrate, I modified your example accordingly:

public class TabPaneRemoveSelected extends Application {

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

    public static class MyTabSkin extends TabPaneSkin {

        public MyTabSkin(TabPane pane) {
            super(pane);
            pane.getTabs().addListener(this::tabsChanged);
        }

        protected void tabsChanged(Change<? extends Tab> c) {
            while (c.next()) {
                if (c.wasRemoved()) {
                    // lookup all TabHeaderSkins
                    Set<Node> tabHeaders = getSkinnable().lookupAll(".tab");
                    tabHeaders.stream()
                        .filter(p -> p instanceof Parent)
                        .map(p -> (Parent) p)
                        .forEach(p -> {
                            // all children removed indicates being in the process
                            // of being removed
                            if (p.getChildrenUnmodifiable().size() == 0) {
                                // complete removeListeners
                                p.setOnMousePressed(null);
                            }
                        }
                    );

                }
            }
        }

    }

    @Override
    public void start(Stage primaryStage) throws Exception {
        TabPane tabPane = new TabPane() {

            @Override
            protected Skin<?> createDefaultSkin() {
                return new MyTabSkin(this);
            }

        };
        tabPane.getSelectionModel().selectedItemProperty().addListener((ov, oldTab, newTab) -> {
            System.out.println("Tab change: " + oldTab + "/" + newTab );
        });
        Tab tab = new Tab("Test tab");
        Tab second = new Tab("second");
        installHandler(tabPane, tab, second);
        installHandler(tabPane, second);
        System.out.println("Adding tab");
        tabPane.getTabs().addAll(tab, second);
        Group root = new Group(tabPane);
        Scene scene = new Scene(root);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    protected void installHandler(TabPane tabPane, Tab... tab) {
        for (Tab tab2 : tab) {
            tab2.setOnCloseRequest((event) -> {
                System.out.println("Removing tab");
                event.consume();
                //I need to remove tab manually
                tabPane.getTabs().remove(tab2);
            });

        }
    }

}

Upvotes: 1

Related Questions