JDK-8245145 : Spinner: throws IllegalArgumentException when replacing skin
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: openjfx14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-05-17
  • Updated: 2022-12-14
  • Resolved: 2022-12-07
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx20 b11Fixed
Related Reports
Blocks :  
JDK-8241364 - ☂ Cleanup skin implementations to allow switching
Relates :  
JDK-8290844 - Add Skin.install() method
Relates :  
JDK-8294809 - ListenerHelper for managing and disconnecting listeners
Description
following test snippet fails with IllegalStateException:

    @Test
    public void testSpinnerSkin() {
        Spinner<?> spinner = new Spinner<>();
        spinner.setSkin(new SpinnerSkin<>(spinner));
        spinner.setSkin(new SpinnerSkin<>(spinner));
    }
    
Exception:

java.lang.IllegalArgumentException: Children: duplicate children added: parent = Spinner@2b40ff9c[styleClass=spinner]
	at javafx.graphics/javafx.scene.Parent$3.onProposedChange(Parent.java:560)
	at javafx.base/com.sun.javafx.collections.VetoableListDecorator.add(VetoableListDecorator.java:206)
	at javafx.controls/javafx.scene.control.skin.SpinnerSkin.<init>(SpinnerSkin.java:112)
	at javafx.controls/test.javafx.scene.control.skin.SkinIssuesTest.testSpinnerSkin(SkinIssuesTest.java:92)

turned up working on JDK-8244531  

don't quite understand why it is a problem here but not in ComboXX (which may or may not have an editor) - they also have editors, any ideas?

NOTE: the fix requires both ListenerHelper JDK-8294809 and Skin.install() JDK-8290844 changes.
Comments
Changeset: 07e693b5 Author: Andy Goryachev <angorya@openjdk.org> Date: 2022-12-07 16:10:02 +0000 URL: https://git.openjdk.org/jfx/commit/07e693b52f54dd92ee055de7c51f173902dda583
07-12-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jfx/pull/904 Date: 2022-09-28 15:08:19 +0000
28-11-2022
Requires new Skin.install() JDK-8290844 to fix properly.
28-09-2022
> Clearing out the child list might be the best solution Yes, I think so too. I wonder if other there are any controls that might have the same problem of accumulating debris when skins are switched...
19-05-2020
ahh, it's done in ComboBoxBaseSkin .. forgot to look up the super chain ;) Clearing out the child list might be the best solution: there are also the incr/decr buttons which are simply added - they will not throw (because they are different instances) but look like never being removed, every skin adding their own, accumulating ... which indeed happens (if adding the textField only if not yet contained) @Test public void testSpinnerChildren() { Spinner<?> spinner = new Spinner<>(); spinner.setSkin(new SpinnerSkin<>(spinner)); int childCount = spinner.getChildrenUnmodifiable().size(); spinner.setSkin(new SpinnerSkin<>(spinner)); assertEquals(childCount, spinner.getChildrenUnmodifiable().size()); } testing all skins for stable childCount reveals 11 more that pile up children .. dooh
19-05-2020
> I wonder if other there are any controls that might have the same problem of accumulating debris when skins are switched With JDK-8244531 we can write a quick test for all skins (in controls package) - that where the 11 in my previous comment came up: import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import static org.junit.Assert.*; import static test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory.*; import static javafx.scene.control.ControlShim.*; import javafx.scene.control.Control; /** * Quick check: cleanup of children on replacing skin */ @RunWith(Parameterized.class) public class SkinChildCountTest { private Control control; private Class<Control> controlClass; // --------- test @Test public void testControlChildren() { installDefaultSkin(control); int childCount = control.getChildrenUnmodifiable().size(); replaceSkin(control); assertEquals(childCount, control.getChildrenUnmodifiable().size()); } //--------- parameterized // Note: name property not supported before junit 4.11 @Parameterized.Parameters //(name = "{index}: {0} ") public static Collection<Object[]> data() { return asArrays(getControlClasses()); } public SkinChildCountTest(Class<Control> control) { this.controlClass = control; } //------------ setup @Before public void setup() { assertNotNull(controlClass); Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> { if (throwable instanceof RuntimeException) { throw (RuntimeException)throwable; } else { Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable); } }); control = createControl(controlClass); } @After public void cleanup() { Thread.currentThread().setUncaughtExceptionHandler(null); } }
19-05-2020
This looks like a bug in SpinnerSkin. It adds the Spinner's editor (a TextField) as a child node when the skin is constructed. This also makes it a child of the Spinner control (since the control and the skin share the list of children for all skins that extend SkinBase). When a new skin is constructed, the textField is added again. As for why it isn't a problem for ComboBox, I see the following in the ComboBoxBaseSkin constructor: getChildren().clear(); I don't know what the implications would be in doing the same thing in the SpinnerSkin, but that might be a solution. Another solution might be to check whether the editor is already a child: if (!getChildren().contains(textField) { getChildren().add(textField); }
18-05-2020