Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8094684

[ListView] ListView getSelectionModel().isEmpty() == false even when no more items in the corresponding ListView

    XMLWordPrintable

    Details

      Description

      When a listView backing list becomes empty after some items removals calling getSelectionModel().isEmpty() returns false.

      One can use the following test case for recreate.

      package javafx.scene.control;

      import static org.junit.Assert.assertTrue;
      import javafx.collections.FXCollections;
      import javafx.embed.swing.JFXPanel;

      import org.junit.Before;
      import org.junit.Test;

      public class ListViewTest {

      @Before
      public void init(){
      // WAD for JFX8 to initialize the toolkit, otherwise java.lang.IllegalStateException: Toolkit not initialized
      //
      new JFXPanel();
      }

      @Test
      public void testRemoveAll() {
      ListView<String> listView = new ListView<String>(FXCollections.observableArrayList("one", "two", "three"));

      listView.getSelectionModel().selectFirst();

      listView.getItems().removeAll("one", "two", "three");

      // notify the [null] element on the list
      System.out.println(listView.getSelectionModel().getSelectedItems());

      assertTrue(listView.getSelectionModel().isEmpty());
      }
      }

      After a quick analysis of decompilled sources it looks like this behaviour is connected to the following part of code in

      javafx.scene.control.MultipleSelectionModelBase.shiftSelection(int, int)

      --------------------------------------------------------
      } else if (shift < 0) {
                  for (int i = position; i < selectedIndicesSize; i++) {
                      if ((i + shift) < 0) continue; <--------- 1*
                      boolean selected = selectedIndices.get(i + 1);
                      selectedIndices.set(i + 1 + shift, selected); <--------- it will never update a 0th bit (in connection with [1*] (i+1+shift) > 0 ) in the selectedIndices BitSet so it will always
                                                                                                                             indicate 0th item as selected and thus:
                                                                                                                            -) never return true from isEmpty.
                                                                                                                            -) getSelectionModel().getSelectedItems() will return a collection with a null object inside
                                                                                                                            -) adding items to this listview will keep shifting this null item to the end
                                                                                                                            -) possibly will eventually break logic that depends on sizes of collections (e.g. replace)

                      if (selected) {
                          perm[idx++] = i;
                      }
                  }
      }
      ---------------------------------------------------------

      The code is quite messy imho and I haven't analyzed it more than I needed but I might have come up with two possible fixes

      1 - let the 0th bit be updated by modifying the mentioned code

      //AS IS
      if ((i + shift) < 0) continue;

      //TO BE
      if ((i + shift) < -1) continue;

      2 - (BETTER) considering it's a border case - no more items in a list view - shiftSelection can be avoided by a simple check in

      javafx.scene.control.ListView.ListViewBitSetSelectionModel.updateSelection(Change<? extends T>)

      //AS IS
      } else if (c.wasAdded() || c.wasRemoved()) {
                    int shift = c.wasAdded() ? c.getAddedSize() : -c.getRemovedSize();
                    shiftSelection(c.getFrom(), shift);
      }

      to :

      //TO BE
      } else if (c.wasAdded() || c.wasRemoved()) {
                    shiftSelection(c.getFrom(), c.getAddedSize());
      } else if (c.wasRemoved()) {
                 if (getItemCount() == 0) {
                              // all items were removed from the model
                              clearSelection();
                  } else {
                      shiftSelection(c.getFrom(), -c.getRemovedSize());
                  }
      }

      Please let me know what you think.

      Thanks.

        Attachments

          Activity

            People

            Assignee:
            jgiles Jonathan Giles
            Reporter:
            pbazanjfx Piotr Bazan (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Imported: