Search code examples
javafxpseudo-class

PseudoClass: notification from states broken on setting custom state


Below is an example that doesn't behave like I expect it to behave - would like to find out if the behavior or my expectation is wrong (can well be that I do something foolishly wrong ;).

What it does:

  • it implements a custom button with a custom pseudo-class very much the same way as in the code example in the java doc
  • it registers a listener on the observableSet returned by the button's getPseudoClassStates() that logs the active states on change
  • it has ui (a simple button) that toggles the special state of the custom button

Expected behavior: notifications on interaction with the custom button via mouse/keyboard (hover, focused, armed ... )

Actual behavior: same as expected if the special state is not set, no notification after the special state has been set once

Questions:

  • bug or feature?
  • how to change the setup to be notified always?

The example:

public class PseudoStateNotification extends Application {
    private SpecialButton specialButton;
    private SetChangeListener sl;

    public static class SpecialButton extends Button {
        private static PseudoClass SPECIAL = PseudoClass.getPseudoClass("special");
        private BooleanProperty special = new SimpleBooleanProperty(this, "special", false) {

            @Override
            protected void invalidated() {
                pseudoClassStateChanged(SPECIAL, get());
            }

        };

        public SpecialButton(String text) {
            super(text);
        }

        public void setSpecial(boolean sp) {
            special.set(sp);
        }

        public boolean isSpecial() {
            return special.get();
        }

        public BooleanProperty specialProperty() {
            return special;
        }
    }

    private Parent createContent() {
        sl = change -> LOG.info("pseudo-changed: " + change.getSet());
        specialButton =  new SpecialButton("custom buttom");
        specialButton.getPseudoClassStates().addListener(sl);
        BorderPane pane = new BorderPane(specialButton);
        Button toggle = new Button("toggle special state");
        toggle.setOnAction(e -> {
            specialButton.setSpecial(!specialButton.isSpecial());
        });
        pane.setBottom(toggle);
        return pane;
    }

    @Override
    public void start(Stage stage) throws Exception {
        stage.setScene(new Scene(createContent(), 200, 200));
        URL uri = getClass().getResource("pseudo.css");
        stage.getScene().getStylesheets().add(uri.toExternalForm());
        stage.setTitle(FXUtils.version());
        stage.show();
    }

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

    @SuppressWarnings("unused")
    private static final Logger LOG = Logger
            .getLogger(PseudoStateNotification.class.getName());

}

The stylesheet pseudo.css:

.button:special {
  -fx-text-fill: red;
}

Update:

As nicely tracked in Jose's answer, the underlying reason is that ObservaleSet<PseudoClass> Node.getPseudoClassStates() returns a free-flying unmodifiable wrapper around the states which makes the returned set garbage-collectable. In my book that's a bug - comparable api as for instance TableView.getVisibleLeafColumns() is implemented correctly and keeps a strong reference to the wrapper. The hack-around is to keep a strong reference to the set in client code.


Solution

  • It is not a matter of a custom state, but it is the usual case of weak listeners that are garbage collected at some point: your SetChangeListener will be gone after some time.

    You can verify it by not clicking at all in the toggle button. Just hover a few times the button, and click on it. After some point you won't get any notification.

    This is for instance a sequence of changes:

    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [hover, pressed, focused]
    INFO: pseudo-changed: [hover, pressed, focused, armed]
    INFO: pseudo-changed: [hover, focused, armed]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [hover, pressed, focused]
    INFO: pseudo-changed: [hover, pressed, focused, armed]
    INFO: pseudo-changed: [hover, focused, armed]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [hover, pressed, focused]
    INFO: pseudo-changed: [hover, pressed, focused, armed]
    INFO: pseudo-changed: [hover, focused, armed]
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: [hover, focused]
    

    After the last one I don't get notifications anymore.

    To avoid this all we need to do is hold a strong reference of the observable set of pseudo classes:

    private ObservableSet<PseudoClass> states;
    
    private Parent createContent() {
        sl = change -> LOG.info("pseudo-changed: " + change.getSet());
        specialButton =  new SpecialButton("custom buttom");
        states = specialButton.getPseudoClassStates();
        states.addListener(sl);
        ...
    }
    

    Now it will work with any state change, both from the button or from the toggle button.

    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: []
    INFO: pseudo-changed: [special]
    INFO: pseudo-changed: [hover, special]
    INFO: pseudo-changed: [hover, pressed, special]
    INFO: pseudo-changed: [hover, pressed, focused, special]
    ...
    INFO: pseudo-changed: [focused, special]
    INFO: pseudo-changed: [special]
    INFO: pseudo-changed: []
    INFO: pseudo-changed: [hover]
    ...
    

    EDIT

    While finalize has been deprecated, it still can be used to check that the listener is gc'ed in the first scenario:

    specialButton.getPseudoClassStates().addListener(new SetChangeListener<PseudoClass>() {
            @Override
            public void onChanged(Change<? extends PseudoClass> change) {
                LOG.info("pseudo-changed: " + change.getSet());
            }
    
            @Override
            protected void finalize() throws Throwable {
                super.finalize();
                LOG.info("SetChangeListener finalized by gc");
            }
        });
    

    So in the above test, you will get:

    ...
    INFO: pseudo-changed: [hover, focused]
    INFO: pseudo-changed: [focused]
    INFO: pseudo-changed: [hover, focused]
    INFO: SetChangeListener finalized by gc
    

    and no more notifications after that, as mentioned before.

    However, if you hold the strong reference, even if you call explicitly System.gc(), the listener won't be gc'ed.

    It is also worth checking the FXCollection::UnmodifiableObservableSet implementation, where a WeakSetChangeListener is used, while holding a reference to the listener itself:

    private SetChangeListener<E> listener;
    private void initListener() {
            if (listener == null) {
                listener = c -> {
                    callObservers(new SetAdapterChange<E>(UnmodifiableObservableSet.this, c));
                };
                this.backingSet.addListener(new WeakSetChangeListener<E>(listener));
            }
        }
    

    This means that the reference private SetChangeListener<E> listener; is not really needed.