Using ControlsFxVisualizer causes node graph inconsistencies

[ntherrien]

Consider the following simplified code:

TextInputDialog dialog = new TextInputDialog(viewModel.selectedServerUriProperty().get());
                dialog.setTitle("Edit Server URI Dialog");
                dialog.setHeaderText("Edit the server URI");
                dialog.setContentText("Server URI:");
                dialog.getEditor().setMinWidth(300);

                // Setup Validation
                ServerUriValidator serverUriValidator = new ServerUriValidator(dialog.contentTextProperty());
                final Button btOk = (Button) dialog.getDialogPane().lookupButton(ButtonType.OK);
                btOk.disableProperty().bind(serverUriValidator.getValidationStatus().validProperty().not());

                ValidationVisualizer visualizer = new ControlsFxVisualizer();
                visualizer.initVisualization(serverUriValidator.getValidationStatus(), dialog.getEditor(), true);

                // Go
                Optional<String> result = dialog.showAndWait();
                result.ifPresent(uriString -> controller.edit(viewModel.selectedServerUriProperty().get(), uriString));

If you comment out the ValidationVisualizer, the code runs fine.

If you uncomment the ValidationVisualizer, the "is already inside a scene-graph and cannot be set as root" IllegalArgumentException is shown (see at the very end of this issue).

Alternatively, if you use this alternate implementation, there is no issue:

 TextInputDialog dialog = new TextInputDialog(viewModel.selectedServerUriProperty().get());
                dialog.setTitle("Edit Server URI Dialog");
                dialog.setHeaderText("Edit the server URI");
                dialog.setContentText("Server URI:");
                dialog.getEditor().setMinWidth(300);

                // Setup Validation
                ServerUriValidator serverUriValidator = new ServerUriValidator(dialog.getEditor().textProperty());
                final Button btOk = (Button) dialog.getDialogPane().lookupButton(ButtonType.OK);
                btOk.disableProperty().bind(serverUriValidator.getValidationStatus().validProperty().not());

//                ValidationVisualizer visualizer = new ControlsFxVisualizer();
//                visualizer.initVisualization(serverUriValidator.getValidationStatus(), dialog.getEditor(), true);

                ValidationDecoration decoration = new GraphicValidationDecoration();
                serverUriValidator.getValidationStatus().validProperty().not().addListener((observable, oldValue, newValue) -> {
                    Optional<de.saxsys.mvvmfx.utils.validation.ValidationMessage> highestMessage = serverUriValidator.getValidationStatus().getHighestMessage();
                    if(highestMessage.isPresent()) {
                        if (newValue) {
                            decoration.applyValidationDecoration(new ValidationMessage() {
                                @Override
                                public String getText() {
                                    return highestMessage.get().getMessage();
                                }

                                @Override
                                public Severity getSeverity() {
                                    switch(highestMessage.get().getSeverity()) {
                                        case ERROR:
                                            return Severity.ERROR;
                                        case WARNING:
                                            return Severity.WARNING;
                                    }
                                    return null;
                                }

                                @Override
                                public Control getTarget() {
                                    return dialog.getEditor();
                                }
                            });
                        }
                    }
                    else {
                        decoration.removeDecorations(dialog.getEditor());
                    }

                });

                // Go
                Optional<String> result = dialog.showAndWait();
                result.ifPresent(uriString -> controller.edit(viewModel.selectedServerUriProperty().get(), uriString));

Note: I also have been experiencing issues even when using the full MVVM pattern. Especially when the controls were under tab containers. The parent window would be unexpectedly resized/reset compared to when no visualizer were set. It seems that success in using this tool depends on how the parent node structure is defined.

There is something wrong with how the node graph is set up with the visualizers and I hope the simple example above will be helpful in diagnosing the issue. Once this simple example works, I'm pretty sure other issues under the MVVM pattern (Tab Container) will be gone too.

* SOLUTION **

Consider the following code in ValidationVisualizerBase

    public void initVisualization(ValidationStatus result, Control control, boolean required) {
        if (required) {
            this.applyRequiredVisualization(control, required);
        }

        this.applyVisualization(control, result.getHighestMessage(), required);
        result.getMessages().addListener((c) -> {
            while(c.next()) {
                Platform.runLater(() -> {
                    this.applyVisualization(control, result.getHighestMessage(), required);
                });
            }

        });
    }

The first two steps calling "applyRequiredVisualization" and "applyVisualization" are both calling JavaFX code too early. They should be wrapped in a runLater just like the third one.

What happens is that we are calling this JavaFx code to show something on a scene graph that is still being built. The initialize method is usually called from code that is building the scene.

I have tried this fix locally and it tests good.

FULL STACK TRACE FOR FIRST EXAMPLE:

13:35:43.622 [JavaFX Application Thread] INFO v.s.c.j.d.c.ConnectionDialogController - Loading server list configuration file: config/default/serverlist.json 13:35:49.035 [JavaFX Application Thread] ERROR d.s.m.utils.commands.DelegateCommand - Exception in Command Execution java.lang.IllegalArgumentException: DialogPane@4747f46d[styleClass=root dialog-pane text-input-dialog]is already inside a scene-graph and cannot be set as root at javafx.scene.Scene$9.invalidated(Scene.java:1100) at javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:111) at javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:146) at javafx.scene.Scene.setRoot(Scene.java:1072) at javafx.scene.control.HeavyweightDialog.showAndWait(HeavyweightDialog.java:160) at javafx.scene.control.Dialog.showAndWait(Dialog.java:341) at vesta.simulator.common.javafx.dialog.connection.ConnectionDialog$5.action(ConnectionDialog.java:121) at de.saxsys.mvvmfx.utils.commands.DelegateCommand.callActionAndSynthesizeServiceRun(DelegateCommand.java:157) at de.saxsys.mvvmfx.utils.commands.DelegateCommand.execute(DelegateCommand.java:134) at vesta.simulator.common.javafx.command.DelegateCommand.execute(DelegateCommand.java:40) at vesta.simulator.common.javafx.dialog.connection.ConnectionDialogView.onEdit(ConnectionDialogView.java:60) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at sun.reflect.misc.Trampoline.invoke(MethodUtil.java:71) at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at sun.reflect.misc.MethodUtil.invoke(MethodUtil.java:275) at javafx.fxml.FXMLLoader$MethodHandler.invoke(FXMLLoader.java:1769) at javafx.fxml.FXMLLoader$ControllerMethodEventHandler.handle(FXMLLoader.java:1657) at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86) at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238) at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191) at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74) at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49) at javafx.event.Event.fireEvent(Event.java:198) at javafx.scene.Node.fireEvent(Node.java:8413) at javafx.scene.control.Button.fire(Button.java:185) at com.sun.javafx.scene.control.behavior.ButtonBehavior.mouseReleased(ButtonBehavior.java:182) at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:96) at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:89) at com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218) at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80) at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238) at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191) at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74) at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54) at javafx.event.Event.fireEvent(Event.java:198) at javafx.scene.Scene$MouseHandler.process(Scene.java:3757) at javafx.scene.Scene$MouseHandler.access$1500(Scene.java:3485) at javafx.scene.Scene.impl_processMouseEvent(Scene.java:1762) at javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2494) at com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:381) at com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:295) at java.security.AccessController.doPrivileged(Native Method) at com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$handleMouseEvent$354(GlassViewEventHandler.java:417) at com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:389) at com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:416) at com.sun.glass.ui.View.handleMouseEvent(View.java:555) at com.sun.glass.ui.View.notifyMouse(View.java:937) at com.sun.glass.ui.win.WinApplication._enterNestedEventLoopImpl(Native Method) at com.sun.glass.ui.win.WinApplication._enterNestedEventLoop(WinApplication.java:218) at com.sun.glass.ui.Application.enterNestedEventLoop(Application.java:511) at com.sun.glass.ui.EventLoop.enter(EventLoop.java:107) at com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(QuantumToolkit.java:583) at javafx.stage.Stage.showAndWait(Stage.java:474) at vesta.simulator.nena.i3.lis.admin.ui.LisAdministrationApplication.showConnectionDialog(LisAdministrationApplication.java:111) at vesta.simulator.nena.i3.lis.admin.ui.LisAdministrationApplication.start(LisAdministrationApplication.java:93) at com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$162(LauncherImpl.java:863) at com.sun.javafx.application.PlatformImpl.lambda$runAndWait$175(PlatformImpl.java:326) at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295) at java.security.AccessController.doPrivileged(Native Method) at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294) at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95) at com.sun.glass.ui.win.WinApplication._runLoop(Native Method) at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191) at java.lang.Thread.run(Thread.java:748)

[ntherrien]

@lestard assign this issue to me if you would like me to submit the suggested bug fix.

[manuel-mauky]

Thanks for the help. Pull-Requests are allways highly appreciated :-)

[ntherrien]

Hi @lestard , adding the runLater was easy, but I want to add a test class for ControlsFxVisualizer. Not sure how I can mock "Platform.runLater". I see we have a couple of test libraries in the parent pom, do you have a preferred method of mocking static methods? I've used PowerMock in the past, but I thought perhaps you knew of a better, modern, JavaFx testing library?

[ntherrien]

Also, no problem on the help! I was very happy to come accross this library. It brought my Javafx gui code from spaghetti monster to testable pleasure-to-work-with. I thank you and the community for sharing and am happy to contribute.

[manuel-mauky]

I'm not a fan of mocking static methods with tools like PowerMock. And I think that it would falsify the meaning of the tests. My prefered method to to test Platform.runLater is to add some "waiting" steps in the Test code and let Platform.runLater untouched. We have the https://sialcasa.github.io/mvvmFX/javadoc/1.7.0/mvvmfx-testing-utils/de/saxsys/mvvmfx/testingutils/FxTestingUtils.html#waitForUiThread-long- util method that makes sure that all other "runLater" functions are executed before the test execution progresses.

[ntherrien]

@lestard, I submitted the fix. I followed your advice for testing runLater. Using waitForUiThread wasn't enough, however, because sometimes the Runnable was being executed before the test had time to verify anything. To improve on the tooling already in FxTestingUtils, i have added a method which allows to pause and resume the Fx platform thread in order to do some validation.

Let me know if that is a good fit.