8273339: IOOBE with ListChangeListener added to the selectedItems lis…

…t of a TableView Backport-of: c55931f
openjdk · Jun 29, 2022 · 7d33552 · 7d33552 · openjdk-notifier · Jun 29, 2022
1 parent 97fd2cb
commit 7d33552
Show file tree

Hide file tree

Showing 5 changed files with 148 additions and 7 deletions.
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java
@@ -157,7 +157,7 @@ private void checkState() {
         };
     }
 
-    public static <S> void updateSelectedIndices(MultipleSelectionModelBase<S> sm, ListChangeListener.Change<? extends TablePositionBase<?>> c, IntPredicate removeRowFilter) {
+    public static <S> void updateSelectedIndices(MultipleSelectionModelBase<S> sm, boolean isCellSelectionEnabled, ListChangeListener.Change<? extends TablePositionBase<?>> c, IntPredicate removeRowFilter) {
         sm.selectedIndices._beginChange();
 
         while (c.next()) {
@@ -181,14 +181,21 @@ public static <S> void updateSelectedIndices(MultipleSelectionModelBase<S> sm, L
                     .count();
             sm.stopAtomic();
 
-            final int to = c.getFrom() + addedSize;
+            int from = c.getFrom();
+            if (isCellSelectionEnabled && 0 < from && from < c.getList().size()) {
+                // convert origin of change of list of tablePositions
+                // into origin of change of list of rows
+                int tpRow = c.getList().get(from).getRow();
+                from = sm.selectedIndices.indexOf(tpRow);
+            }
+            final int to = from + addedSize;
 
             if (c.wasReplaced()) {
-                sm.selectedIndices._nextReplace(c.getFrom(), to, removed);
+                sm.selectedIndices._nextReplace(from, to, removed);
             } else if (c.wasRemoved()) {
-                sm.selectedIndices._nextRemove(c.getFrom(), removed);
+                sm.selectedIndices._nextRemove(from, removed);
             } else if (c.wasAdded()) {
-                sm.selectedIndices._nextAdd(c.getFrom(), to);
+                sm.selectedIndices._nextAdd(from, to);
             }
         }
         c.reset();

diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
@@ -3028,7 +3028,7 @@ private void fireCustomSelectedCellsListChangeEvent(ListChangeListener.Change<?
             // if such row doesn't have any selected cells
             IntPredicate removeRowFilter = row -> !isCellSelectionEnabled() ||
                     getSelectedCells().stream().noneMatch(tp -> tp.getRow() == row);
-            ControlUtils.updateSelectedIndices(this, c, removeRowFilter);
+            ControlUtils.updateSelectedIndices(this, this.isCellSelectionEnabled(), c, removeRowFilter);
 
             if (isAtomic()) {
                 return;

diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
@@ -3370,7 +3370,7 @@ private void fireCustomSelectedCellsListChangeEvent(ListChangeListener.Change<?
             // if such row doesn't have any selected cells
             IntPredicate removeRowFilter = row -> !isCellSelectionEnabled() ||
                     getSelectedCells().stream().noneMatch(tp -> tp.getRow() == row);
-            ControlUtils.updateSelectedIndices(this, c, removeRowFilter);
+            ControlUtils.updateSelectedIndices(this, this.isCellSelectionEnabled(), c, removeRowFilter);
 
             if (isAtomic()) {
                 return;

diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java
@@ -5635,4 +5635,87 @@ public void test_clearAndSelectChangeMultipleSelectionCellMode() {
         sl.dispose();
     }
 
+    @Test
+    public void test_ChangeToStringKeyboardMultipleSelectionCellMode() {
+        final Thread.UncaughtExceptionHandler exceptionHandler = Thread.currentThread().getUncaughtExceptionHandler();
+        Thread.currentThread().setUncaughtExceptionHandler((t, e) -> fail("We don't expect any exceptions in this test!"));
+
+        TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
+        firstNameCol.setCellValueFactory(new PropertyValueFactory<>("firstName"));
+
+        TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
+        lastNameCol.setCellValueFactory(new PropertyValueFactory<>("lastName"));
+
+        TableView<Person> table = new TableView<>();
+        table.setItems(personTestData);
+        table.getColumns().addAll(firstNameCol, lastNameCol);
+
+        sm = table.getSelectionModel();
+        sm.setCellSelectionEnabled(true);
+        sm.setSelectionMode(SelectionMode.MULTIPLE);
+
+        // Call change::toString
+        table.getSelectionModel().getSelectedItems().addListener((ListChangeListener<Person>) Object::toString);
+
+        StageLoader sl = new StageLoader(table);
+
+        KeyEventFirer keyboard = new KeyEventFirer(table);
+
+        assertEquals(0, sm.getSelectedItems().size());
+
+        sm.select(0, firstNameCol);
+        assertEquals(1, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        keyboard.doKeyPress(KeyCode.RIGHT, KeyModifier.SHIFT);
+        assertEquals(2, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        sm.clearSelection();
+
+        sl.dispose();
+
+        // reset the exception handler
+        Thread.currentThread().setUncaughtExceptionHandler(exceptionHandler);
+    }
+
+    @Test
+    public void test_ChangeToStringMouseMultipleSelectionCellMode() {
+        final Thread.UncaughtExceptionHandler exceptionHandler = Thread.currentThread().getUncaughtExceptionHandler();
+        Thread.currentThread().setUncaughtExceptionHandler((t, e) -> fail("We don't expect any exceptions in this test!"));
+
+        TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
+        firstNameCol.setCellValueFactory(new PropertyValueFactory<>("firstName"));
+
+        TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
+        lastNameCol.setCellValueFactory(new PropertyValueFactory<>("lastName"));
+
+        TableView<Person> table = new TableView<>();
+        table.setItems(personTestData);
+        table.getColumns().addAll(firstNameCol, lastNameCol);
+
+        sm = table.getSelectionModel();
+        sm.setCellSelectionEnabled(true);
+        sm.setSelectionMode(SelectionMode.MULTIPLE);
+
+        // Call change::toString
+        table.getSelectionModel().getSelectedItems().addListener((ListChangeListener<Person>) Object::toString);
+
+        assertEquals(0, sm.getSelectedItems().size());
+
+        sm.select(0, firstNameCol);
+        assertTrue(sm.isSelected(0, firstNameCol));
+        assertEquals(1, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        TableCell<Person, String> cell = (TableCell<Person, String>) VirtualFlowTestUtils.getCell(table, 0, 1);
+        new MouseEventFirer(cell).fireMousePressAndRelease(KeyModifier.getShortcutKey());
+        assertTrue(sm.isSelected(0, firstNameCol));
+        assertTrue(sm.isSelected(0, lastNameCol));
+        assertEquals(2, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        // reset the exception handler
+        Thread.currentThread().setUncaughtExceptionHandler(exceptionHandler);
+    }
 }
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java
@@ -6715,4 +6715,55 @@ public void test_clearAndSelectChangeMultipleSelectionCellMode() {
         assertEquals(1, sm.getSelectedItems().size());
     }
 
+    @Test
+    public void test_ChangeToStringMouseMultipleSelectionCellMode() {
+        final Thread.UncaughtExceptionHandler exceptionHandler = Thread.currentThread().getUncaughtExceptionHandler();
+        Thread.currentThread().setUncaughtExceptionHandler((t, e) -> fail("We don't expect any exceptions in this test!"));
+
+        TreeItem<Person> root = new TreeItem<>(new Person("root", "",""));
+        root.getChildren().setAll(
+                new TreeItem<>(new Person("Jacob", "Smith", "jacob.smith@example.com")),
+                new TreeItem<>(new Person("Isabella", "Johnson", "isabella.johnson@example.com")),
+                new TreeItem<>(new Person("Ethan", "Williams", "ethan.williams@example.com")),
+                new TreeItem<>(new Person("Emma", "Jones", "emma.jones@example.com")),
+                new TreeItem<>(new Person("Michael", "Brown", "michael.brown@example.com")));
+        root.setExpanded(true);
+
+        TreeTableColumn<Person, String> firstNameCol = new TreeTableColumn<>("First Name");
+        firstNameCol.setCellValueFactory(new TreeItemPropertyValueFactory<>("firstName"));
+
+        TreeTableColumn<Person, String> lastNameCol = new TreeTableColumn<>("Last Name");
+        lastNameCol.setCellValueFactory(new TreeItemPropertyValueFactory<>("lastName"));
+
+        TreeTableColumn<Person, String> emailCol = new TreeTableColumn<>("Email");
+        emailCol.setCellValueFactory(new TreeItemPropertyValueFactory<>("email"));
+
+        TreeTableView<Person> table = new TreeTableView<>(root);
+        table.getColumns().addAll(firstNameCol, lastNameCol, emailCol);
+        sm = table.getSelectionModel();
+        sm.setSelectionMode(SelectionMode.MULTIPLE);
+        sm.setCellSelectionEnabled(true);
+
+        // Call change::toString
+        table.getSelectionModel().getSelectedItems().addListener((ListChangeListener<TreeItem<Person>>) Object::toString);
+
+        assertEquals(0, sm.getSelectedItems().size());
+
+        sm.select(1, firstNameCol);
+        assertTrue(sm.isSelected(1, firstNameCol));
+        assertEquals(1, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        TreeTableCell<Person, String> cell = (TreeTableCell<Person, String>) VirtualFlowTestUtils.getCell(table, 1, 1);
+        MouseEventFirer mouse = new MouseEventFirer(cell);
+        mouse.fireMousePressAndRelease(KeyModifier.getShortcutKey());
+        assertTrue(sm.isSelected(1, firstNameCol));
+        assertTrue(sm.isSelected(1, lastNameCol));
+        assertEquals(2, sm.getSelectedCells().size());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        // reset the exception handler
+        Thread.currentThread().setUncaughtExceptionHandler(exceptionHandler);
+    }
+
 }