Skip to content

Commit e866a6c

Browse files
author
Andy Goryachev
committed
8296413: Tree/TableView with null focus model throws NPE in queryAccessibleAttribute()
Reviewed-by: arapte
1 parent 0dbc448 commit e866a6c

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, 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
@@ -827,10 +827,22 @@ private void possiblySetStyle(String styleCandidate) {
827827
@Override
828828
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
829829
switch (attribute) {
830-
case ROW_INDEX: return getIndex();
831-
case COLUMN_INDEX: return columnIndex;
832-
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
833-
default: return super.queryAccessibleAttribute(attribute, parameters);
830+
case ROW_INDEX:
831+
return getIndex();
832+
case COLUMN_INDEX:
833+
return columnIndex;
834+
case SELECTED:
835+
if (isInCellSelectionMode()) {
836+
return isSelected();
837+
} else {
838+
if(getTableRow() == null) {
839+
return null;
840+
} else {
841+
return getTableRow().isSelected();
842+
}
843+
}
844+
default:
845+
return super.queryAccessibleAttribute(attribute, parameters);
834846
}
835847
}
836848

‎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
@@ -1791,10 +1791,15 @@ public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object...
17911791
@SuppressWarnings("unchecked")
17921792
ObservableList<TableRow<S>> rows = (ObservableList<TableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
17931793
List<Node> selection = new ArrayList<>();
1794-
for (TableRow<S> row : rows) {
1795-
@SuppressWarnings("unchecked")
1796-
ObservableList<Node> cells = (ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
1797-
if (cells != null) selection.addAll(cells);
1794+
if (rows != null) {
1795+
for (TableRow<S> row: rows) {
1796+
@SuppressWarnings("unchecked")
1797+
ObservableList<Node> cells =
1798+
(ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
1799+
if (cells != null) {
1800+
selection.addAll(cells);
1801+
}
1802+
}
17981803
}
17991804
return FXCollections.observableArrayList(selection);
18001805
}

‎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, 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
@@ -852,10 +852,22 @@ private void possiblySetStyle(String styleCandidate) {
852852
@Override
853853
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
854854
switch (attribute) {
855-
case ROW_INDEX: return getIndex();
856-
case COLUMN_INDEX: return columnIndex;
857-
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
858-
default: return super.queryAccessibleAttribute(attribute, parameters);
855+
case ROW_INDEX:
856+
return getIndex();
857+
case COLUMN_INDEX:
858+
return columnIndex;
859+
case SELECTED:
860+
if (isInCellSelectionMode()) {
861+
return isSelected();
862+
} else {
863+
if (getTableRow() == null) {
864+
return null;
865+
} else {
866+
return getTableRow().isSelected();
867+
}
868+
}
869+
default:
870+
return super.queryAccessibleAttribute(attribute, parameters);
859871
}
860872
}
861873

‎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;
@@ -84,15 +61,33 @@
8461
import javafx.collections.MapChangeListener;
8562
import javafx.collections.ObservableList;
8663
import javafx.collections.WeakListChangeListener;
64+
import javafx.css.CssMetaData;
65+
import javafx.css.PseudoClass;
66+
import javafx.css.Styleable;
67+
import javafx.css.StyleableDoubleProperty;
68+
import javafx.css.StyleableProperty;
69+
import javafx.css.converter.SizeConverter;
8770
import javafx.event.Event;
8871
import javafx.event.EventHandler;
8972
import javafx.event.EventType;
73+
import javafx.event.WeakEventHandler;
9074
import javafx.scene.AccessibleAttribute;
9175
import javafx.scene.AccessibleRole;
9276
import javafx.scene.Node;
77+
import javafx.scene.control.skin.TreeTableViewSkin;
9378
import javafx.scene.layout.Region;
9479
import javafx.util.Callback;
9580

81+
import com.sun.javafx.collections.MappingChange;
82+
import com.sun.javafx.collections.NonIterableChange;
83+
import com.sun.javafx.scene.control.Properties;
84+
import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;
85+
import com.sun.javafx.scene.control.SelectedCellsMap;
86+
import com.sun.javafx.scene.control.TableColumnComparatorBase;
87+
import com.sun.javafx.scene.control.behavior.TableCellBehavior;
88+
import com.sun.javafx.scene.control.behavior.TableCellBehaviorBase;
89+
import com.sun.javafx.scene.control.behavior.TreeTableCellBehavior;
90+
9691
/**
9792
* The TreeTableView control is designed to visualize an unlimited number of rows
9893
* 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, 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
@@ -1048,6 +1048,9 @@ private boolean isCellFocused(int row) {
10481048
switch (attribute) {
10491049
case FOCUS_ITEM: {
10501050
TableFocusModel<M,?> fm = getFocusModel();
1051+
if (fm == null) {
1052+
return null;
1053+
}
10511054
int focusedIndex = fm.getFocusedIndex();
10521055
if (focusedIndex == -1) {
10531056
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
@@ -5991,7 +5991,6 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
59915991
assertEquals(FXCollections.observableArrayList(), result);
59925992
}
59935993

5994-
@Ignore("JDK-8296413")
59955994
@Test
59965995
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
59975996
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
@@ -79,6 +79,7 @@
7979
import javafx.scene.control.Cell;
8080
import javafx.scene.control.FocusModel;
8181
import javafx.scene.control.IndexedCell;
82+
import javafx.scene.control.Label;
8283
import javafx.scene.control.MultipleSelectionModel;
8384
import javafx.scene.control.MultipleSelectionModelBaseShim;
8485
import javafx.scene.control.ScrollBar;
@@ -7134,17 +7135,47 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
71347135
assertEquals(FXCollections.observableArrayList(), result);
71357136
}
71367137

7137-
@Ignore("JDK-8296413")
71387138
@Test
7139-
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
7139+
public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel_Placeholder() {
71407140
treeTableView.setSelectionModel(null);
71417141
stageLoader = new StageLoader(treeTableView);
71427142

71437143
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
7144+
// Should be a placeholder label
7145+
assertTrue(result instanceof Label);
7146+
}
7147+
7148+
@Test
7149+
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
7150+
// with rows
7151+
treeTableView.setRoot(new TreeItem("Root"));
7152+
treeTableView.getRoot().setExpanded(true);
7153+
for (int i = 0; i < 4; i++) {
7154+
TreeItem parent = new TreeItem("item - " + i);
7155+
treeTableView.getRoot().getChildren().add(parent);
7156+
}
71447157

7145-
// TODO it seems to return a Label; possibly the placeholder label.
7146-
// we need to check whether it's what is expected, whether TableView should use the same logic
7147-
// And also check whether it *is* a placeholder label, see TreeTableView:2146
7158+
// with columns
7159+
for (int i = 0; i < 10; i++) {
7160+
TreeTableColumn<String, String> c = new TreeTableColumn<>("C" + i);
7161+
c.setCellValueFactory(value -> new SimpleStringProperty(value.getValue().getValue()));
7162+
treeTableView.getColumns().add(c);
7163+
}
7164+
7165+
treeTableView.setFocusModel(null);
7166+
7167+
stageLoader = new StageLoader(treeTableView);
7168+
7169+
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
7170+
assertNull(result);
7171+
}
7172+
7173+
@Test
7174+
public void testQueryAccessibleAttributeFocusItemWithNullFocusModelPlaceholder() {
7175+
treeTableView.setFocusModel(null);
7176+
stageLoader = new StageLoader(treeTableView);
7177+
7178+
Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
71487179
assertNull(result);
71497180
}
71507181
}

‎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)