On Thu, 9 Apr 2020 09:58:27 GMT, Jesper Skov <github.com+2720909+js...@openjdk.org> wrote:
> Make the two ways of associating a ToggleButton with a ToggleGroup interact > correctly. > > This fixes https://bugs.openjdk.java.net/browse/JDK-8198402 The fix looks correct to me. Have you run all tests to ensure no regressions? I left a couple inline comments. modules/javafx.controls/src/main/java/javafx/scene/control/ToggleButton.java line 196: > 195: private ObjectProperty<ToggleGroup> toggleGroup; > 196: @Override > 197: public final void setToggleGroup(ToggleGroup value) { This is unrelated to the fix. The changes in this file should be reverted. modules/javafx.controls/src/test/java/test/javafx/scene/control/ToggleButtonTest.java line 270: > 269: } catch (InterruptedException e) { > 270: System.err.println("InterruptedException occurred during > Thread.sleep()"); > 271: } An exception here should cause the test to fail. You can use `Assert.fail` or else re-throw the exception as an `AssertionFailedError`. modules/javafx.controls/src/main/java/javafx/scene/control/ToggleGroup.java line 74: > 73: while (c.next()) { > 74: List<Toggle> addedToggles = c.getAddedSubList(); > 75: This can be declared as final. ------------- PR: https://git.openjdk.java.net/jfx/pull/167