JDK-8267781 : Tree-/TableView: incorrect notification from selectedIndices
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: jfx16
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2021-05-26
  • Updated: 2024-06-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
Relates :  
Description
happens when cell selection is enabled with multiple mode

failing test:

    @Test public void selectRowWhenInMultipleCellSelectionModeNotification() {
        model.setSelectionMode(SelectionMode.MULTIPLE);
        model.setCellSelectionEnabled(true);
        model.clearSelection();
        List<Change> changes = new ArrayList<>();
        model.getSelectedIndices().addListener((Change<? extends Integer> c) -> {
            changes.add(c);
            // Note: simple logging points to the reason for the error: 
            // System.out.println(c);
        });
        model.select(1);
        assertEquals("indices must fire single change: ", 1, changes.size());
    }

The underlying reason is ControlUtils.updateSelectedIndices: its task is to map the notifications received from selectedCells to notifications of selectedIndices. It fails by sending multiple notifications for the same row (probably with incorrect to/from coordinates)

Yeah, the change coordinates fired by updateSelectedIndices are definitely wrong:

    // the change originated in selectedCells, coordinates in a flat list
    public static <S> void updateSelectedIndices(MultipleSelectionModelBase<S> sm, ListChangeListener.Change<? extends TablePositionBase<?>> c) {
        sm.selectedIndices._beginChange();

        while (c.next()) {
            // [.. ] do some magic to change the indices under their feet ..

            // error: c.getFrom is in coordinates of the list of cells
            final int to = c.getFrom() ... ;
           // error: mark a change multiple time (for each next)
             if (c.wasReplaced()) {
                sm.selectedIndices._nextReplace(c.getFrom(), to, removed);
            } ... 
        }


Comments
Another aspect of this issue is an inconsistency in notification when un/selecting columns in the same row: A: select(row, column1) expected and actual - fire a single change of type add B: select(row, column2) expected: do not fire (at least not of type add), actual: fires a single change of type add C: clearSelection(row, column2) expected and actual - do not fire for consistency, B/C should either both fire or not at all (the selectedIndices didn't change) - below is a unit test to catch this expectation Firing an add/remove would be definitely wrong - if we _really_ (don't know) need to, we could fire an update in both cases: technically that would denote replacing the row index with the same. It's a bit of stretching, though. Test code: /** * Test for consistency of select/clear notification */ @Test public void clearColumnsWhenInMultipleCellSelectionModeNotification() { model.setSelectionMode(SelectionMode.MULTIPLE); model.setCellSelectionEnabled(true); model.clearSelection(); int row = 1; // select column model.select(row, tableView.getColumns().get(0)); // prepare measuring List<Change> changes = new ArrayList<>(); model.getSelectedIndices().addListener((Change<? extends Integer> c) -> { changes.add(c); }); // select another column model.select(row, tableView.getColumns().get(1)); int onSelect = changes.size(); changes.clear(); // clear selection of a selected column model.clearSelection(row, tableView.getColumns().get(1)); assertEquals("notification count on clear must be same as on select", onSelect, changes.size()); }
01-09-2022