Skip to content

Commit cb545cc

Browse files
author
Jeanette Winzenburg
committed
8253634: TreeCell/Skin: misbehavior on switching skin
Reviewed-by: arapte
1 parent 51c09e5 commit cb545cc

File tree

4 files changed

+65
-41
lines changed

4 files changed

+65
-41
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeCellSkin.java

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2017, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -32,18 +32,16 @@
3232
import java.util.WeakHashMap;
3333

3434
import com.sun.javafx.scene.control.behavior.BehaviorBase;
35-
import javafx.beans.InvalidationListener;
36-
import javafx.beans.Observable;
3735
import javafx.beans.property.DoubleProperty;
3836
import javafx.beans.value.WritableValue;
3937
import javafx.geometry.HPos;
4038
import javafx.geometry.VPos;
4139
import javafx.scene.Node;
4240
import javafx.scene.control.Control;
43-
import javafx.scene.control.ListView;
4441
import javafx.scene.control.TreeCell;
4542
import javafx.scene.control.TreeItem;
4643
import javafx.scene.control.TreeView;
44+
import javafx.scene.layout.Region;
4745
import javafx.css.StyleableDoubleProperty;
4846
import javafx.css.StyleableProperty;
4947
import javafx.css.CssMetaData;
@@ -85,7 +83,6 @@ public class TreeCellSkin<T> extends CellSkinBase<TreeCell<T>> {
8583
private static final Map<TreeView<?>, Double> maxDisclosureWidthMap = new WeakHashMap<TreeView<?>, Double>();
8684

8785

88-
8986
/***************************************************************************
9087
* *
9188
* Private fields *
@@ -96,10 +93,6 @@ public class TreeCellSkin<T> extends CellSkinBase<TreeCell<T>> {
9693
private TreeItem<?> treeItem;
9794
private final BehaviorBase<TreeCell<T>> behavior;
9895

99-
private double fixedCellSize;
100-
private boolean fixedCellSizeEnabled;
101-
102-
10396

10497
/***************************************************************************
10598
* *
@@ -129,31 +122,9 @@ public TreeCellSkin(TreeCell<T> control) {
129122
getSkinnable().requestLayout();
130123
});
131124
registerChangeListener(control.textProperty(), e -> getSkinnable().requestLayout());
132-
133-
setupTreeViewListeners();
134-
}
135-
136-
private void setupTreeViewListeners() {
137-
TreeView<T> treeView = getSkinnable().getTreeView();
138-
if (treeView == null) {
139-
getSkinnable().treeViewProperty().addListener(new InvalidationListener() {
140-
@Override public void invalidated(Observable observable) {
141-
getSkinnable().treeViewProperty().removeListener(this);
142-
setupTreeViewListeners();
143-
}
144-
});
145-
} else {
146-
this.fixedCellSize = treeView.getFixedCellSize();
147-
this.fixedCellSizeEnabled = fixedCellSize > 0;
148-
registerChangeListener(treeView.fixedCellSizeProperty(), e -> {
149-
this.fixedCellSize = getSkinnable().getTreeView().getFixedCellSize();
150-
this.fixedCellSizeEnabled = fixedCellSize > 0;
151-
});
152-
}
153125
}
154126

155127

156-
157128
/***************************************************************************
158129
* *
159130
* Properties *
@@ -277,7 +248,8 @@ public final DoubleProperty indentProperty() {
277248

278249
/** {@inheritDoc} */
279250
@Override protected double computeMinHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
280-
if (fixedCellSizeEnabled) {
251+
double fixedCellSize = getFixedCellSize();
252+
if (fixedCellSize > 0) {
281253
return fixedCellSize;
282254
}
283255

@@ -288,7 +260,8 @@ public final DoubleProperty indentProperty() {
288260

289261
/** {@inheritDoc} */
290262
@Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
291-
if (fixedCellSizeEnabled) {
263+
double fixedCellSize = getFixedCellSize();
264+
if (fixedCellSize > 0) {
292265
return fixedCellSize;
293266
}
294267

@@ -305,7 +278,8 @@ public final DoubleProperty indentProperty() {
305278

306279
/** {@inheritDoc} */
307280
@Override protected double computeMaxHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
308-
if (fixedCellSizeEnabled) {
281+
double fixedCellSize = getFixedCellSize();
282+
if (fixedCellSize > 0) {
309283
return fixedCellSize;
310284
}
311285

@@ -340,6 +314,10 @@ public final DoubleProperty indentProperty() {
340314
return pw;
341315
}
342316

317+
private double getFixedCellSize() {
318+
TreeView<?> treeView = getSkinnable().getTreeView();
319+
return treeView != null ? treeView.getFixedCellSize() : Region.USE_COMPUTED_SIZE;
320+
}
343321

344322

345323
/***************************************************************************

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeCellTest.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -39,6 +39,7 @@
3939
import org.junit.Ignore;
4040
import org.junit.Test;
4141

42+
import static javafx.scene.control.ControlShim.*;
4243
import static test.com.sun.javafx.scene.control.infrastructure.ControlTestUtils.*;
4344
import static org.junit.Assert.*;
4445

@@ -730,4 +731,27 @@ private final class FocusModelMock extends FocusModel {
730731
TreeCell cell = new TreeCell();
731732
cell.setSkin(new TreeCellSkin(cell));
732733
}
734+
735+
/**
736+
* Test that min/max/pref height respect fixedCellSize.
737+
* Sanity test when fixing JDK-8253634.
738+
*/
739+
@Test
740+
public void testTreeCellHeights() {
741+
TreeCell<Object> cell = new TreeCell<>();
742+
TreeView<Object> treeView = new TreeView<>();
743+
cell.updateTreeView(treeView);
744+
installDefaultSkin(cell);
745+
treeView.setFixedCellSize(100);
746+
assertEquals("pref height must be fixedCellSize",
747+
treeView.getFixedCellSize(),
748+
cell.prefHeight(-1), 1);
749+
assertEquals("min height must be fixedCellSize",
750+
treeView.getFixedCellSize(),
751+
cell.minHeight(-1), 1);
752+
assertEquals("max height must be fixedCellSize",
753+
treeView.getFixedCellSize(),
754+
cell.maxHeight(-1), 1);
755+
}
756+
733757
}

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import javafx.scene.control.ListCell;
4343
import javafx.scene.control.ListView;
4444
import javafx.scene.control.ToolBar;
45+
import javafx.scene.control.TreeCell;
46+
import javafx.scene.control.TreeView;
4547
import javafx.scene.layout.Pane;
4648
import javafx.scene.layout.VBox;
4749
import javafx.scene.shape.Rectangle;
@@ -56,6 +58,32 @@ public class SkinCleanupTest {
5658
private Stage stage;
5759
private Pane root;
5860

61+
// ------------------ TreeCell
62+
63+
@Test
64+
public void testTreeCellReplaceTreeViewWithNull() {
65+
TreeCell<Object> cell = new TreeCell<>();
66+
TreeView<Object> treeView = new TreeView<>();
67+
cell.updateTreeView(treeView);
68+
installDefaultSkin(cell);
69+
cell.updateTreeView(null);
70+
// 8253634: updating the old treeView must not throw NPE in skin
71+
treeView.setFixedCellSize(100);
72+
}
73+
74+
@Test
75+
public void testTreeCellPrefHeightOnReplaceTreeView() {
76+
TreeCell<Object> cell = new TreeCell<>();
77+
cell.updateTreeView(new TreeView<>());
78+
installDefaultSkin(cell);
79+
TreeView<Object> treeView = new TreeView<>();
80+
treeView.setFixedCellSize(100);
81+
cell.updateTreeView(treeView);
82+
assertEquals("fixed cell set to value of new treeView",
83+
cell.getTreeView().getFixedCellSize(),
84+
cell.prefHeight(-1), 1);
85+
}
86+
5987
// ------------------ ListCell
6088

6189
@Test

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,10 @@
4242

4343
import javafx.scene.control.Accordion;
4444
import javafx.scene.control.ButtonBar;
45-
import javafx.scene.control.ChoiceBox;
4645
import javafx.scene.control.ColorPicker;
4746
import javafx.scene.control.ComboBox;
4847
import javafx.scene.control.Control;
4948
import javafx.scene.control.DatePicker;
50-
import javafx.scene.control.ListCell;
51-
import javafx.scene.control.ListView;
5249
import javafx.scene.control.MenuBar;
5350
import javafx.scene.control.MenuButton;
5451
import javafx.scene.control.Pagination;
@@ -63,8 +60,6 @@
6360
import javafx.scene.control.TableView;
6461
import javafx.scene.control.TextArea;
6562
import javafx.scene.control.TextField;
66-
import javafx.scene.control.ToolBar;
67-
import javafx.scene.control.TreeCell;
6863
import javafx.scene.control.TreeTableRow;
6964
import javafx.scene.control.TreeTableView;
7065
import javafx.scene.control.TreeView;
@@ -128,7 +123,6 @@ public static Collection<Object[]> data() {
128123
TextArea.class,
129124
// @Ignore("8240506")
130125
TextField.class,
131-
TreeCell.class,
132126
TreeTableRow.class,
133127
TreeTableView.class,
134128
TreeView.class

0 commit comments

Comments
 (0)