Skip to content

Commit a094252

Browse files
author
Johan Vos
committed
8296413: Tree/TableView with null focus model throws NPE in queryAccessibleAttribute()
Reviewed-by: kcr Backport-of: e866a6c
1 parent 5ff6556 commit a094252

File tree

8 files changed

+209
-54
lines changed

8 files changed

+209
-54
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/TableCell.java

+17-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2023, 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
@@ -816,10 +816,22 @@ private void possiblySetStyle(String styleCandidate) {
816816
@Override
817817
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
818818
switch (attribute) {
819-
case ROW_INDEX: return getIndex();
820-
case COLUMN_INDEX: return columnIndex;
821-
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
822-
default: return super.queryAccessibleAttribute(attribute, parameters);
819+
case ROW_INDEX:
820+
return getIndex();
821+
case COLUMN_INDEX:
822+
return columnIndex;
823+
case SELECTED:
824+
if (isInCellSelectionMode()) {
825+
return isSelected();
826+
} else {
827+
if(getTableRow() == null) {
828+
return null;
829+
} else {
830+
return getTableRow().isSelected();
831+
}
832+
}
833+
default:
834+
return super.queryAccessibleAttribute(attribute, parameters);
823835
}
824836
}
825837

‎modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2023, 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
@@ -1790,10 +1790,15 @@ public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object...
17901790
@SuppressWarnings("unchecked")
17911791
ObservableList<TableRow<S>> rows = (ObservableList<TableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
17921792
List<Node> selection = new ArrayList<>();
1793-
for (TableRow<S> row : rows) {
1794-
@SuppressWarnings("unchecked")
1795-
ObservableList<Node> cells = (ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
1796-
if (cells != null) selection.addAll(cells);
1793+
if (rows != null) {
1794+
for (TableRow<S> row: rows) {
1795+
@SuppressWarnings("unchecked")
1796+
ObservableList<Node> cells =
1797+
(ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
1798+
if (cells != null) {
1799+
selection.addAll(cells);
1800+
}
1801+
}
17971802
}
17981803
return FXCollections.observableArrayList(selection);
17991804
}

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableCell.java

+17-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2023, 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
@@ -845,10 +845,22 @@ private void possiblySetStyle(String styleCandidate) {
845845
@Override
846846
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
847847
switch (attribute) {
848-
case ROW_INDEX: return getIndex();
849-
case COLUMN_INDEX: return columnIndex;
850-
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
851-
default: return super.queryAccessibleAttribute(attribute, parameters);
848+
case ROW_INDEX:
849+
return getIndex();
850+
case COLUMN_INDEX:
851+
return columnIndex;
852+
case SELECTED:
853+
if (isInCellSelectionMode()) {
854+
return isSelected();
855+
} else {
856+
if (getTableRow() == null) {
857+
return null;
858+
} else {
859+
return getTableRow().isSelected();
860+
}
861+
}
862+
default:
863+
return super.queryAccessibleAttribute(attribute, parameters);
852864
}
853865
}
854866

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java

+30-30
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2023, 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
@@ -25,30 +25,6 @@
2525

2626
package javafx.scene.control;
2727

28-
import com.sun.javafx.collections.MappingChange;
29-
import com.sun.javafx.collections.NonIterableChange;
30-
import com.sun.javafx.scene.control.Properties;
31-
import com.sun.javafx.scene.control.SelectedCellsMap;
32-
33-
import com.sun.javafx.scene.control.behavior.TableCellBehavior;
34-
import com.sun.javafx.scene.control.behavior.TableCellBehaviorBase;
35-
import com.sun.javafx.scene.control.behavior.TreeTableCellBehavior;
36-
37-
import javafx.beans.property.DoubleProperty;
38-
import javafx.css.CssMetaData;
39-
import javafx.css.PseudoClass;
40-
41-
import javafx.css.converter.SizeConverter;
42-
import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;
43-
import com.sun.javafx.scene.control.TableColumnComparatorBase;
44-
45-
import javafx.css.Styleable;
46-
import javafx.css.StyleableDoubleProperty;
47-
import javafx.css.StyleableProperty;
48-
import javafx.event.WeakEventHandler;
49-
50-
import javafx.scene.control.skin.TreeTableViewSkin;
51-
5228
import java.lang.ref.SoftReference;
5329
import java.lang.ref.WeakReference;
5430
import java.util.ArrayList;
@@ -69,6 +45,7 @@
6945
import javafx.beans.InvalidationListener;
7046
import javafx.beans.WeakInvalidationListener;
7147
import javafx.beans.property.BooleanProperty;
48+
import javafx.beans.property.DoubleProperty;
7249
import javafx.beans.property.ObjectProperty;
7350
import javafx.beans.property.ObjectPropertyBase;
7451
import javafx.beans.property.ReadOnlyIntegerProperty;
@@ -85,15 +62,33 @@
8562
import javafx.collections.MapChangeListener;
8663
import javafx.collections.ObservableList;
8764
import javafx.collections.WeakListChangeListener;
65+
import javafx.css.CssMetaData;
66+
import javafx.css.PseudoClass;
67+
import javafx.css.Styleable;
68+
import javafx.css.StyleableDoubleProperty;
69+
import javafx.css.StyleableProperty;
70+
import javafx.css.converter.SizeConverter;
8871
import javafx.event.Event;
8972
import javafx.event.EventHandler;
9073
import javafx.event.EventType;
74+
import javafx.event.WeakEventHandler;
9175
import javafx.scene.AccessibleAttribute;
9276
import javafx.scene.AccessibleRole;
9377
import javafx.scene.Node;
78+
import javafx.scene.control.skin.TreeTableViewSkin;
9479
import javafx.scene.layout.Region;
9580
import javafx.util.Callback;
9681

82+
import com.sun.javafx.collections.MappingChange;
83+
import com.sun.javafx.collections.NonIterableChange;
84+
import com.sun.javafx.scene.control.Properties;
85+
import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;
86+
import com.sun.javafx.scene.control.SelectedCellsMap;
87+
import com.sun.javafx.scene.control.TableColumnComparatorBase;
88+
import com.sun.javafx.scene.control.behavior.TableCellBehavior;
89+
import com.sun.javafx.scene.control.behavior.TableCellBehaviorBase;
90+
import com.sun.javafx.scene.control.behavior.TreeTableCellBehavior;
91+
9792
/**
9893
* The TreeTableView control is designed to visualize an unlimited number of rows
9994
* of data, broken out into columns. The TreeTableView control is conceptually
@@ -2129,12 +2124,17 @@ public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object...
21292124
*/
21302125
case SELECTED_ITEMS: {
21312126
@SuppressWarnings("unchecked")
2132-
ObservableList<TreeTableRow<S>> rows = (ObservableList<TreeTableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
2127+
ObservableList<TreeTableRow<S>> rows =
2128+
(ObservableList<TreeTableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
21332129
List<Node> selection = new ArrayList<>();
2134-
for (TreeTableRow<S> row : rows) {
2135-
@SuppressWarnings("unchecked")
2136-
ObservableList<Node> cells = (ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
2137-
if (cells != null) selection.addAll(cells);
2130+
if (rows != null) {
2131+
for (TreeTableRow<S> row: rows) {
2132+
@SuppressWarnings("unchecked")
2133+
List<Node> cells = (List<Node>)row.queryAccessibleAttribute(attribute, parameters);
2134+
if (cells != null) {
2135+
selection.addAll(cells);
2136+
}
2137+
}
21382138
}
21392139
return FXCollections.observableArrayList(selection);
21402140
}

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2023, 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
@@ -1017,6 +1017,9 @@ private boolean isCellFocused(int row) {
10171017
switch (attribute) {
10181018
case FOCUS_ITEM: {
10191019
TableFocusModel<M,?> fm = getFocusModel();
1020+
if (fm == null) {
1021+
return null;
1022+
}
10201023
int focusedIndex = fm.getFocusedIndex();
10211024
if (focusedIndex == -1) {
10221025
if (placeholderRegion != null && placeholderRegion.isVisible()) {

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2023, 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
@@ -5952,7 +5952,6 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
59525952
assertEquals(FXCollections.observableArrayList(), result);
59535953
}
59545954

5955-
@Ignore("JDK-8296413")
59565955
@Test
59575956
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
59585957
table.getItems().addAll("1", "2");

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

+37-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2023, 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
@@ -106,6 +106,7 @@
106106
import javafx.scene.control.Cell;
107107
import javafx.scene.control.FocusModel;
108108
import javafx.scene.control.IndexedCell;
109+
import javafx.scene.control.Label;
109110
import javafx.scene.control.MultipleSelectionModel;
110111
import javafx.scene.control.MultipleSelectionModelBaseShim;
111112
import javafx.scene.control.SelectionMode;
@@ -7099,17 +7100,47 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
70997100
assertEquals(FXCollections.observableArrayList(), result);
71007101
}
71017102

7102-
@Ignore("JDK-8296413")
71037103
@Test
7104-
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
7104+
public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel_Placeholder() {
71057105
treeTableView.setSelectionModel(null);
71067106
stageLoader = new StageLoader(treeTableView);
71077107

71087108
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
7109+
// Should be a placeholder label
7110+
assertTrue(result instanceof Label);
7111+
}
71097112

7110-
// TODO it seems to return a Label; possibly the placeholder label.
7111-
// we need to check whether it's what is expected, whether TableView should use the same logic
7112-
// And also check whether it *is* a placeholder label, see TreeTableView:2146
7113+
@Test
7114+
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
7115+
// with rows
7116+
treeTableView.setRoot(new TreeItem("Root"));
7117+
treeTableView.getRoot().setExpanded(true);
7118+
for (int i = 0; i < 4; i++) {
7119+
TreeItem parent = new TreeItem("item - " + i);
7120+
treeTableView.getRoot().getChildren().add(parent);
7121+
}
7122+
7123+
// with columns
7124+
for (int i = 0; i < 10; i++) {
7125+
TreeTableColumn<String, String> c = new TreeTableColumn<>("C" + i);
7126+
c.setCellValueFactory(value -> new SimpleStringProperty(value.getValue().getValue()));
7127+
treeTableView.getColumns().add(c);
7128+
}
7129+
7130+
treeTableView.setFocusModel(null);
7131+
7132+
stageLoader = new StageLoader(treeTableView);
7133+
7134+
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
7135+
assertNull(result);
7136+
}
7137+
7138+
@Test
7139+
public void testQueryAccessibleAttributeFocusItemWithNullFocusModelPlaceholder() {
7140+
treeTableView.setFocusModel(null);
7141+
stageLoader = new StageLoader(treeTableView);
7142+
7143+
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
71137144
assertNull(result);
71147145
}
71157146
}

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

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package test.javafx.scene.control.skin;
26+
27+
import static org.junit.Assert.assertNotNull;
28+
import java.util.Collection;
29+
import java.util.List;
30+
import org.junit.After;
31+
import org.junit.Before;
32+
import org.junit.Test;
33+
import org.junit.runner.RunWith;
34+
import org.junit.runners.Parameterized;
35+
import javafx.scene.AccessibleAttribute;
36+
import javafx.scene.Node;
37+
import javafx.scene.control.Control;
38+
import test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory;
39+
40+
/**
41+
* Tests whether queryAccessibleAttribute() in every Control handles all of the
42+
* AccessibleAttribute values without throwing an exception.
43+
*/
44+
@RunWith(Parameterized.class)
45+
public class QueryAccessibleAttributeTest {
46+
private Class<Node> nodeClass;
47+
private Node node;
48+
49+
@Parameterized.Parameters
50+
public static Collection<Object[]> nodesUnderTest() {
51+
List<Class<Control>> cs = ControlSkinFactory.getControlClasses();
52+
return ControlSkinFactory.asArrays(cs);
53+
}
54+
55+
public QueryAccessibleAttributeTest(Class<Node> nodeClass) {
56+
this.nodeClass = nodeClass;
57+
}
58+
59+
@Before
60+
public void setup() {
61+
Thread.currentThread().setUncaughtExceptionHandler((thread, err) -> {
62+
if (err instanceof RuntimeException) {
63+
throw (RuntimeException)err;
64+
} else {
65+
Thread.currentThread().getThreadGroup().uncaughtException(thread, err);
66+
}
67+
});
68+
69+
node = createNode(nodeClass);
70+
assertNotNull(node);
71+
}
72+
73+
@After
74+
public void cleanup() {
75+
Thread.currentThread().setUncaughtExceptionHandler(null);
76+
}
77+
78+
protected static <T extends Node> T createNode(Class<T> controlClass) {
79+
try {
80+
return controlClass.getDeclaredConstructor().newInstance();
81+
} catch (Exception e) {
82+
throw new RuntimeException(e);
83+
}
84+
}
85+
86+
@Test
87+
public void queryAllAttributes() {
88+
for (AccessibleAttribute a: AccessibleAttribute.values()) {
89+
// should throw no exceptions
90+
Object val = node.queryAccessibleAttribute(a);
91+
}
92+
}
93+
}

0 commit comments

Comments
 (0)