RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE

[Jeanette Winzenburg]

On Sat, 8 Jan 2022 00:17:36 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> This PR fixes a bunch of NPEs when a null `SelectionModel` or `FocusModel` is set on a `ListView`.
> 
> The following NPEs are fixed (all are also covered by exactly one test case):
> NPEs with null selection model:
> - Mouse click on a `ListCell`
> - SPACE key press
> - KP_UP (arrow up) key press
> - HOME key press
> - END key press
> - BACK_SLASH + CTRL key press
> 
> NPEs with null focus model:
> - SPACE key press
> - Select an items: getSelectionModel().select(1)
> - Clear-Select an item and add one after: listView.getSelectionModel().clearAndSelect(1); listView.getItems().add("3");

Fix and tests look rather complete, NPEs are fixed

There is still one unguarded access to the selectionModel in ListViewSkin.queryAccessibleAttributes: don't know if that's ever reached without actually having a selectionModel?

As we have similar NPEs in all our behaviors (of virtualized controls), we might decided here which behavior we want to support and then apply that decision everywhere. In particular what should happen if we have a null selection and a not-null focusModel with

- activate/edit: that happens on the focusedIndex/Cell with selection only an after-thought
- focus navigation (often ctrl-somekey): is inconsistently either supported (ctrl-end) or not (ctrl-pageDown)

Personally, I think that we should support both - we have a separate focusModel so should use it independently of the selectionModel where possible. A good starter for this issue might be to implement the activate such that it doesn't back out on null selection (just leave out the select statement). The consistent support of navigation might be done in a follow-up issue.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewMouseInputTest.java line 397:

> 395: 
> 396:         loader.dispose();
> 397:     }

the stageloader is not disposed if the test fails - so a safer route is to keep it in a field which is then disposed in  @ after (which we decided to do years ago, but tend to forget occasionally :), see f.i. ListViewKeyInputTest

BTW: good to explicitely create the stageloader and __not__ rely on the one implicitly created by the flowUtils!

modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java line 2196:

> 2194: 
> 2195:         StageLoader stageLoader = new StageLoader(listView);
> 2196: 

just to not forget: stageloader again

modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java line 2208:

> 2206: 
> 2207:         StageLoader stageLoader = new StageLoader(listView);
> 2208: 

just to not forget: stageloader again

-------------

Changes requested by fastegal (Reviewer).

PR: https://git.openjdk.org/jfx/pull/711