On Fri, 27 Mar 2026 21:55:57 GMT, Philemon Hilscher <[email protected]> wrote:
> This fixes the issue adressed here https://bugs.openjdk.org/browse/JDK-8222454 > > The added TreeTableViewTests were failing without the main code changes. I > added the other tests to ensure the consistent functionality of related cell > based components. > The method getItemCount() needed to be moved to CellBehaviorBase to do the > necessary index check in doSelect() below. All subclasses now implement this > method based on the cell container. > > There is still some refactoring potential here, to reduce code duplications > and ensure consistent behavior of all cell based components. This could be > done in a follow-up PR. Changes requested by mhanl (Committer). modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java line 167: > 165: > 166: protected int getIndex() { > 167: return getNode() != null ? getNode().getIndex() : -1; Can `getNode()` return null? modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java line 207: > 205: final T cell = getNode(); > 206: > 207: final Control cellContainer = getCellContainer(); Again, we should null check the `cellContainer` first. Like my other comment suggested. You should move `final T cell = getNode();` after the null check then. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java line 210: > 208: > 209: int count = getItemCount(); > 210: if (cell.getIndex() >= count) return; I think we should rather use the `if` with the braces. if (cell.getIndex() >= count) { return; } modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableCellBehaviorBase.java line 110: > 108: if (! tableCell.contains(x, y)) return; > 109: > 110: final Control tableView = getCellContainer(); I think this should be the very first check here, like other cell classes do. Otherwise `getItemCount` can produce a `NullPointer`. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TreeTableRowBehavior.java line 56: > 54: > 55: /** {@inheritDoc} */ > 56: @Override protected int getItemCount() { `@Override` should be above the method ------------- PR Review: https://git.openjdk.org/jfx/pull/2129#pullrequestreview-4025628709 PR Review Comment: https://git.openjdk.org/jfx/pull/2129#discussion_r3004889545 PR Review Comment: https://git.openjdk.org/jfx/pull/2129#discussion_r3004903624 PR Review Comment: https://git.openjdk.org/jfx/pull/2129#discussion_r3004890663 PR Review Comment: https://git.openjdk.org/jfx/pull/2129#discussion_r3004901573 PR Review Comment: https://git.openjdk.org/jfx/pull/2129#discussion_r3004899485
