changeset 6351:7a8fde19001f

RT-35857: Unexpected interaction between ListView's SelectionModel getSelecteditems() and removing items Reviewed-by: Martin Sladecek
author jgiles
date Mon, 24 Feb 2014 10:32:51 +1300
parents 1e1033a2eb1c
children cce238b1b880
files modules/base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java modules/controls/src/test/java/javafx/scene/control/ListViewTest.java modules/controls/src/test/java/javafx/scene/control/TableViewTest.java modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java
diffstat 5 files changed, 100 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/modules/base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java	Mon Feb 24 10:26:25 2014 +1300
+++ b/modules/base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java	Mon Feb 24 10:32:51 2014 +1300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2014, 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
@@ -27,6 +27,8 @@
 
 import javafx.collections.ModifiableObservableListBase;
 import com.sun.javafx.collections.NonIterableChange.SimplePermutationChange;
+
+import java.util.BitSet;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
@@ -168,31 +170,39 @@
     @Override
     public boolean removeAll(Collection<?> c) {
         beginChange();
-        boolean modified = false;
+        BitSet bs = new BitSet(c.size());
         for (int i = 0; i < size(); ++i) {
             if (c.contains(get(i))) {
-                remove(i);
-                --i;
-                modified = true;
+                bs.set(i);
+            }
+        }
+        if (!bs.isEmpty()) {
+            int cur = size();
+            while ((cur = bs.previousSetBit(cur - 1)) >= 0) {
+                remove(cur);
             }
         }
         endChange();
-        return modified;
+        return !bs.isEmpty();
     }
 
     @Override
     public boolean retainAll(Collection<?> c) {
         beginChange();
-        boolean modified = false;
+        BitSet bs = new BitSet(c.size());
         for (int i = 0; i < size(); ++i) {
             if (!c.contains(get(i))) {
-                remove(i);
-                --i;
-                modified = true;
+                bs.set(i);
+            }
+        }
+        if (!bs.isEmpty()) {
+            int cur = size();
+            while ((cur = bs.previousSetBit(cur - 1)) >= 0) {
+                remove(cur);
             }
         }
         endChange();
-        return modified;
+        return !bs.isEmpty();
     }
 
     private SortHelper helper;
--- a/modules/controls/src/test/java/javafx/scene/control/ListViewTest.java	Mon Feb 24 10:26:25 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/ListViewTest.java	Mon Feb 24 10:32:51 2014 +1300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -809,6 +809,22 @@
         assertEquals("bbc", listView.getSelectionModel().getSelectedItem());
     }
 
+    @Test public void test_rt35857() {
+        ObservableList<String> fxList = FXCollections.observableArrayList("A", "B", "C");
+        final ListView<String> listView = new ListView<String>(fxList);
+
+        listView.getSelectionModel().select(0);
+
+        ObservableList<String> selectedItems = listView.getSelectionModel().getSelectedItems();
+        assertEquals(1, selectedItems.size());
+        assertEquals("A", selectedItems.get(0));
+
+        listView.getItems().removeAll(selectedItems);
+        assertEquals(2, fxList.size());
+        assertEquals("B", fxList.get(0));
+        assertEquals("C", fxList.get(1));
+    }
+
     private int rt_35889_cancel_count = 0;
     @Test public void test_rt35889() {
         final ListView<String> textFieldListView = new ListView<String>();
--- a/modules/controls/src/test/java/javafx/scene/control/TableViewTest.java	Mon Feb 24 10:26:25 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TableViewTest.java	Mon Feb 24 10:32:51 2014 +1300
@@ -2677,4 +2677,20 @@
         // This should result in an IOOBE, but didn't until this bug was fixed
         selectedCellsSeq.subList(from, to);
     }
+
+    @Test public void test_rt35857() {
+        ObservableList<String> fxList = FXCollections.observableArrayList("A", "B", "C");
+        final TableView<String> tableView = new TableView<String>(fxList);
+
+        tableView.getSelectionModel().select(0);
+
+        ObservableList<String> selectedItems = tableView.getSelectionModel().getSelectedItems();
+        assertEquals(1, selectedItems.size());
+        assertEquals("A", selectedItems.get(0));
+
+        tableView.getItems().removeAll(selectedItems);
+        assertEquals(2, fxList.size());
+        assertEquals("B", fxList.get(0));
+        assertEquals("C", fxList.get(1));
+    }
 }
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Mon Feb 24 10:26:25 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Mon Feb 24 10:32:51 2014 +1300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -3303,4 +3303,26 @@
 
         assertTrue(treeView.getSortOrder().isEmpty());
     }
+
+    @Test public void test_rt35857() {
+        TreeItem<String> root = new TreeItem<>("Root");
+        root.setExpanded(true);
+        TreeItem a = new TreeItem("A");
+        TreeItem b = new TreeItem("B");
+        TreeItem c = new TreeItem("C");
+        root.getChildren().setAll(a, b, c);
+
+        final TreeTableView<String> treeTableView = new TreeTableView<String>(root);
+
+        treeTableView.getSelectionModel().select(1);
+
+        ObservableList<TreeItem<String>> selectedItems = treeTableView.getSelectionModel().getSelectedItems();
+        assertEquals(1, selectedItems.size());
+        assertEquals("A", selectedItems.get(0).getValue());
+
+        root.getChildren().removeAll(selectedItems);
+        assertEquals(2, root.getChildren().size());
+        assertEquals("B", root.getChildren().get(0).getValue());
+        assertEquals("C", root.getChildren().get(1).getValue());
+    }
 }
--- a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Mon Feb 24 10:26:25 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Mon Feb 24 10:32:51 2014 +1300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -1650,6 +1650,28 @@
         assertEquals("bbc", treeView.getSelectionModel().getSelectedItem().getValue());
     }
 
+    @Test public void test_rt35857() {
+        TreeItem<String> root = new TreeItem<>("Root");
+        root.setExpanded(true);
+        TreeItem a = new TreeItem("A");
+        TreeItem b = new TreeItem("B");
+        TreeItem c = new TreeItem("C");
+        root.getChildren().setAll(a, b, c);
+
+        final TreeView<String> treeTableView = new TreeView<String>(root);
+
+        treeTableView.getSelectionModel().select(1);
+
+        ObservableList<TreeItem<String>> selectedItems = treeTableView.getSelectionModel().getSelectedItems();
+        assertEquals(1, selectedItems.size());
+        assertEquals("A", selectedItems.get(0).getValue());
+
+        root.getChildren().removeAll(selectedItems);
+        assertEquals(2, root.getChildren().size());
+        assertEquals("B", root.getChildren().get(0).getValue());
+        assertEquals("C", root.getChildren().get(1).getValue());
+    }
+
     private int rt_35889_cancel_count = 0;
     @Test public void test_rt35889() {
         TreeItem a = new TreeItem("a");