diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListCellSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListCellSkin.java index f053d2aec16..56cc55b6bf6 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListCellSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListCellSkin.java @@ -26,17 +26,13 @@ package javafx.scene.control.skin; import com.sun.javafx.scene.control.behavior.BehaviorBase; -import javafx.beans.InvalidationListener; -import javafx.beans.Observable; +import com.sun.javafx.scene.control.behavior.ListCellBehavior; + import javafx.geometry.Orientation; -import javafx.scene.Node; -import javafx.scene.control.Accordion; -import javafx.scene.control.Button; import javafx.scene.control.Control; import javafx.scene.control.ListCell; import javafx.scene.control.ListView; - -import com.sun.javafx.scene.control.behavior.ListCellBehavior; +import javafx.scene.layout.Region; /** * Default skin implementation for the {@link ListCell} control. @@ -52,12 +48,8 @@ public class ListCellSkin extends CellSkinBase> { * * **************************************************************************/ - private double fixedCellSize; - private boolean fixedCellSizeEnabled; private final BehaviorBase> behavior; - - /*************************************************************************** * * * Constructors * @@ -77,31 +69,8 @@ public ListCellSkin(ListCell control) { // install default input map for the ListCell control behavior = new ListCellBehavior<>(control); // control.setInputMap(behavior.getInputMap()); - - setupListeners(); } - private void setupListeners() { - ListView listView = getSkinnable().getListView(); - if (listView == null) { - getSkinnable().listViewProperty().addListener(new InvalidationListener() { - @Override public void invalidated(Observable observable) { - getSkinnable().listViewProperty().removeListener(this); - setupListeners(); - } - }); - } else { - this.fixedCellSize = listView.getFixedCellSize(); - this.fixedCellSizeEnabled = fixedCellSize > 0; - registerChangeListener(listView.fixedCellSizeProperty(), e -> { - this.fixedCellSize = getSkinnable().getListView().getFixedCellSize(); - this.fixedCellSizeEnabled = fixedCellSize > 0; - }); - } - } - - - /*************************************************************************** * * * Public API * @@ -127,7 +96,8 @@ private void setupListeners() { /** {@inheritDoc} */ @Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } @@ -140,7 +110,8 @@ private void setupListeners() { /** {@inheritDoc} */ @Override protected double computeMinHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } @@ -149,10 +120,15 @@ private void setupListeners() { /** {@inheritDoc} */ @Override protected double computeMaxHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) { - if (fixedCellSizeEnabled) { + double fixedCellSize = getFixedCellSize(); + if (fixedCellSize > 0) { return fixedCellSize; } - return super.computeMaxHeight(width, topInset, rightInset, bottomInset, leftInset); } + + private double getFixedCellSize() { + ListView listView = getSkinnable().getListView(); + return listView != null ? listView.getFixedCellSize() : Region.USE_COMPUTED_SIZE; + } } diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java index dac47622647..f581be84dea 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java @@ -42,6 +42,7 @@ import org.junit.Ignore; import org.junit.Test; +import static javafx.scene.control.ControlShim.*; import static test.com.sun.javafx.scene.control.infrastructure.ControlTestUtils.*; import static org.junit.Assert.*; @@ -723,4 +724,28 @@ private final class FocusModelMock extends FocusModel { ListCell cell = new ListCell(); cell.setSkin(new ListCellSkin(cell)); } + + /** + * Test that min/max/pref height respect fixedCellSize. + * Sanity test when fixing JDK-8246745. + */ + @Test + public void testListCellHeights() { + ListCell cell = new ListCell<>(); + ListView listView = new ListView<>(); + cell.updateListView(listView); + installDefaultSkin(cell); + listView.setFixedCellSize(100); + assertEquals("pref height must be fixedCellSize", + listView.getFixedCellSize(), + cell.prefHeight(-1), 1); + assertEquals("min height must be fixedCellSize", + listView.getFixedCellSize(), + cell.minHeight(-1), 1); + assertEquals("max height must be fixedCellSize", + listView.getFixedCellSize(), + cell.maxHeight(-1), 1); + } + + } diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java index 3c3f485fb1b..42989debb6f 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java @@ -39,6 +39,7 @@ import javafx.scene.control.Button; import javafx.scene.control.ChoiceBox; import javafx.scene.control.Control; +import javafx.scene.control.ListCell; import javafx.scene.control.ListView; import javafx.scene.control.ToolBar; import javafx.scene.layout.Pane; @@ -55,6 +56,32 @@ public class SkinCleanupTest { private Stage stage; private Pane root; +// ------------------ ListCell + + @Test + public void testListCellReplaceListViewWithNull() { + ListCell cell = new ListCell<>(); + ListView listView = new ListView<>(); + cell.updateListView(listView); + installDefaultSkin(cell); + cell.updateListView(null); + // 8246745: updating the old listView must not throw NPE in skin + listView.setFixedCellSize(100); + } + + @Test + public void testListCellPrefHeightOnReplaceListView() { + ListCell cell = new ListCell<>(); + cell.updateListView(new ListView<>()); + installDefaultSkin(cell); + ListView listView = new ListView<>(); + listView.setFixedCellSize(100); + cell.updateListView(listView); + assertEquals("fixed cell set to value of new listView", + cell.getListView().getFixedCellSize(), + cell.prefHeight(-1), 1); + } + //-------------- listView @Test diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java index dc510d6301d..c3bd5acc490 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java @@ -110,7 +110,6 @@ public static Collection data() { ColorPicker.class, ComboBox.class, DatePicker.class, - ListCell.class, MenuBar.class, MenuButton.class, Pagination.class,