Skip to content

Commit

Permalink
8296413: Tree/TableView with null focus model throws NPE in queryAcce…
Browse files Browse the repository at this point in the history
…ssibleAttribute()

Reviewed-by: arapte
  • Loading branch information
Andy Goryachev committed Jan 5, 2023
1 parent 0dbc448 commit e866a6c
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 54 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -827,10 +827,22 @@ private void possiblySetStyle(String styleCandidate) {
@Override
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
switch (attribute) {
case ROW_INDEX: return getIndex();
case COLUMN_INDEX: return columnIndex;
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
default: return super.queryAccessibleAttribute(attribute, parameters);
case ROW_INDEX:
return getIndex();
case COLUMN_INDEX:
return columnIndex;
case SELECTED:
if (isInCellSelectionMode()) {
return isSelected();
} else {
if(getTableRow() == null) {
return null;
} else {
return getTableRow().isSelected();
}
}
default:
return super.queryAccessibleAttribute(attribute, parameters);
}
}

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -1791,10 +1791,15 @@ public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object...
@SuppressWarnings("unchecked")
ObservableList<TableRow<S>> rows = (ObservableList<TableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
List<Node> selection = new ArrayList<>();
for (TableRow<S> row : rows) {
@SuppressWarnings("unchecked")
ObservableList<Node> cells = (ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
if (cells != null) selection.addAll(cells);
if (rows != null) {
for (TableRow<S> row: rows) {
@SuppressWarnings("unchecked")
ObservableList<Node> cells =
(ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
if (cells != null) {
selection.addAll(cells);
}
}
}
return FXCollections.observableArrayList(selection);
}
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -852,10 +852,22 @@ private void possiblySetStyle(String styleCandidate) {
@Override
public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object... parameters) {
switch (attribute) {
case ROW_INDEX: return getIndex();
case COLUMN_INDEX: return columnIndex;
case SELECTED: return isInCellSelectionMode() ? isSelected() : getTableRow().isSelected();
default: return super.queryAccessibleAttribute(attribute, parameters);
case ROW_INDEX:
return getIndex();
case COLUMN_INDEX:
return columnIndex;
case SELECTED:
if (isInCellSelectionMode()) {
return isSelected();
} else {
if (getTableRow() == null) {
return null;
} else {
return getTableRow().isSelected();
}
}
default:
return super.queryAccessibleAttribute(attribute, parameters);
}
}

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -25,30 +25,6 @@

package javafx.scene.control;

import com.sun.javafx.collections.MappingChange;
import com.sun.javafx.collections.NonIterableChange;
import com.sun.javafx.scene.control.Properties;
import com.sun.javafx.scene.control.SelectedCellsMap;

import com.sun.javafx.scene.control.behavior.TableCellBehavior;
import com.sun.javafx.scene.control.behavior.TableCellBehaviorBase;
import com.sun.javafx.scene.control.behavior.TreeTableCellBehavior;

import javafx.beans.property.DoubleProperty;
import javafx.css.CssMetaData;
import javafx.css.PseudoClass;

import javafx.css.converter.SizeConverter;
import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;
import com.sun.javafx.scene.control.TableColumnComparatorBase;

import javafx.css.Styleable;
import javafx.css.StyleableDoubleProperty;
import javafx.css.StyleableProperty;
import javafx.event.WeakEventHandler;

import javafx.scene.control.skin.TreeTableViewSkin;

import java.lang.ref.SoftReference;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
Expand All @@ -69,6 +45,7 @@
import javafx.beans.InvalidationListener;
import javafx.beans.WeakInvalidationListener;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ObjectPropertyBase;
import javafx.beans.property.ReadOnlyIntegerProperty;
Expand All @@ -84,15 +61,33 @@
import javafx.collections.MapChangeListener;
import javafx.collections.ObservableList;
import javafx.collections.WeakListChangeListener;
import javafx.css.CssMetaData;
import javafx.css.PseudoClass;
import javafx.css.Styleable;
import javafx.css.StyleableDoubleProperty;
import javafx.css.StyleableProperty;
import javafx.css.converter.SizeConverter;
import javafx.event.Event;
import javafx.event.EventHandler;
import javafx.event.EventType;
import javafx.event.WeakEventHandler;
import javafx.scene.AccessibleAttribute;
import javafx.scene.AccessibleRole;
import javafx.scene.Node;
import javafx.scene.control.skin.TreeTableViewSkin;
import javafx.scene.layout.Region;
import javafx.util.Callback;

import com.sun.javafx.collections.MappingChange;
import com.sun.javafx.collections.NonIterableChange;
import com.sun.javafx.scene.control.Properties;
import com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList;
import com.sun.javafx.scene.control.SelectedCellsMap;
import com.sun.javafx.scene.control.TableColumnComparatorBase;
import com.sun.javafx.scene.control.behavior.TableCellBehavior;
import com.sun.javafx.scene.control.behavior.TableCellBehaviorBase;
import com.sun.javafx.scene.control.behavior.TreeTableCellBehavior;

/**
* The TreeTableView control is designed to visualize an unlimited number of rows
* of data, broken out into columns. The TreeTableView control is conceptually
Expand Down Expand Up @@ -2129,12 +2124,17 @@ public Object queryAccessibleAttribute(AccessibleAttribute attribute, Object...
*/
case SELECTED_ITEMS: {
@SuppressWarnings("unchecked")
ObservableList<TreeTableRow<S>> rows = (ObservableList<TreeTableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
ObservableList<TreeTableRow<S>> rows =
(ObservableList<TreeTableRow<S>>)super.queryAccessibleAttribute(attribute, parameters);
List<Node> selection = new ArrayList<>();
for (TreeTableRow<S> row : rows) {
@SuppressWarnings("unchecked")
ObservableList<Node> cells = (ObservableList<Node>)row.queryAccessibleAttribute(attribute, parameters);
if (cells != null) selection.addAll(cells);
if (rows != null) {
for (TreeTableRow<S> row: rows) {
@SuppressWarnings("unchecked")
List<Node> cells = (List<Node>)row.queryAccessibleAttribute(attribute, parameters);
if (cells != null) {
selection.addAll(cells);
}
}
}
return FXCollections.observableArrayList(selection);
}
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -1048,6 +1048,9 @@ private boolean isCellFocused(int row) {
switch (attribute) {
case FOCUS_ITEM: {
TableFocusModel<M,?> fm = getFocusModel();
if (fm == null) {
return null;
}
int focusedIndex = fm.getFocusedIndex();
if (focusedIndex == -1) {
if (placeholderRegion != null && placeholderRegion.isVisible()) {
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -5991,7 +5991,6 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
assertEquals(FXCollections.observableArrayList(), result);
}

@Ignore("JDK-8296413")
@Test
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
table.getItems().addAll("1", "2");
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -79,6 +79,7 @@
import javafx.scene.control.Cell;
import javafx.scene.control.FocusModel;
import javafx.scene.control.IndexedCell;
import javafx.scene.control.Label;
import javafx.scene.control.MultipleSelectionModel;
import javafx.scene.control.MultipleSelectionModelBaseShim;
import javafx.scene.control.ScrollBar;
Expand Down Expand Up @@ -7134,17 +7135,47 @@ public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel() {
assertEquals(FXCollections.observableArrayList(), result);
}

@Ignore("JDK-8296413")
@Test
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
public void testQueryAccessibleAttributeSelectedItemsWithNullSelectionModel_Placeholder() {
treeTableView.setSelectionModel(null);
stageLoader = new StageLoader(treeTableView);

Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
// Should be a placeholder label
assertTrue(result instanceof Label);
}

@Test
public void testQueryAccessibleAttributeFocusItemWithNullFocusModel() {
// with rows
treeTableView.setRoot(new TreeItem("Root"));
treeTableView.getRoot().setExpanded(true);
for (int i = 0; i < 4; i++) {
TreeItem parent = new TreeItem("item - " + i);
treeTableView.getRoot().getChildren().add(parent);
}

// TODO it seems to return a Label; possibly the placeholder label.
// we need to check whether it's what is expected, whether TableView should use the same logic
// And also check whether it *is* a placeholder label, see TreeTableView:2146
// with columns
for (int i = 0; i < 10; i++) {
TreeTableColumn<String, String> c = new TreeTableColumn<>("C" + i);
c.setCellValueFactory(value -> new SimpleStringProperty(value.getValue().getValue()));
treeTableView.getColumns().add(c);
}

treeTableView.setFocusModel(null);

stageLoader = new StageLoader(treeTableView);

Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
assertNull(result);
}

@Test
public void testQueryAccessibleAttributeFocusItemWithNullFocusModelPlaceholder() {
treeTableView.setFocusModel(null);
stageLoader = new StageLoader(treeTableView);

Object result = treeTableView.queryAccessibleAttribute(AccessibleAttribute.FOCUS_ITEM);
assertNull(result);
}
}
@@ -0,0 +1,93 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package test.javafx.scene.control.skin;

import static org.junit.Assert.assertNotNull;
import java.util.Collection;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import javafx.scene.AccessibleAttribute;
import javafx.scene.Node;
import javafx.scene.control.Control;
import test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory;

/**
* Tests whether queryAccessibleAttribute() in every Control handles all of the
* AccessibleAttribute values without throwing an exception.
*/
@RunWith(Parameterized.class)
public class QueryAccessibleAttributeTest {
private Class<Node> nodeClass;
private Node node;

@Parameterized.Parameters
public static Collection<Object[]> nodesUnderTest() {
List<Class<Control>> cs = ControlSkinFactory.getControlClasses();
return ControlSkinFactory.asArrays(cs);
}

public QueryAccessibleAttributeTest(Class<Node> nodeClass) {
this.nodeClass = nodeClass;
}

@Before
public void setup() {
Thread.currentThread().setUncaughtExceptionHandler((thread, err) -> {
if (err instanceof RuntimeException) {
throw (RuntimeException)err;
} else {
Thread.currentThread().getThreadGroup().uncaughtException(thread, err);
}
});

node = createNode(nodeClass);
assertNotNull(node);
}

@After
public void cleanup() {
Thread.currentThread().setUncaughtExceptionHandler(null);
}

protected static <T extends Node> T createNode(Class<T> controlClass) {
try {
return controlClass.getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException(e);
}
}

@Test
public void queryAllAttributes() {
for (AccessibleAttribute a: AccessibleAttribute.values()) {
// should throw no exceptions
Object val = node.queryAccessibleAttribute(a);
}
}
}

1 comment on commit e866a6c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.