diff --git a/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java b/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java --- a/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java +++ b/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java @@ -278,6 +278,12 @@ } @Override public void clearAndSelect(int row) { + // RT-33558 if this method has been called with a given row, and that + // row is the only selected row currently, then this method becomes a no-op. + if (getSelectedIndices().size() == 1 && isSelected(row)) { + return; + } + // RT-32411 We used to call quietClearSelection() here, but this // resulted in the selectedItems and selectedIndices lists never // reporting that they were empty. diff --git a/modules/controls/src/main/java/javafx/scene/control/TableView.java b/modules/controls/src/main/java/javafx/scene/control/TableView.java --- a/modules/controls/src/main/java/javafx/scene/control/TableView.java +++ b/modules/controls/src/main/java/javafx/scene/control/TableView.java @@ -2066,6 +2066,13 @@ } @Override public void clearAndSelect(int row, TableColumn column) { + // RT-33558 if this method has been called with a given row/column + // intersection, and that row/column intersection is the only + // selection currently, then this method becomes a no-op. + if (getSelectedCells().size() == 1 && isSelected(row, column)) { + return; + } + // RT-32411 We used to call quietClearSelection() here, but this // resulted in the selectedItems and selectedIndices lists never // reporting that they were empty. diff --git a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java --- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java +++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java @@ -2204,6 +2204,13 @@ } @Override public void clearAndSelect(int row, TableColumnBase,?> column) { + // RT-33558 if this method has been called with a given row/column + // intersection, and that row/column intersection is the only + // selection currently, then this method becomes a no-op. + if (getSelectedCells().size() == 1 && isSelected(row, column)) { + return; + } + // RT-32411: We used to call quietClearSelection() here, but this // resulted in the selectedItems and selectedIndices lists never // reporting that they were empty. diff --git a/modules/controls/src/test/java/javafx/scene/control/ListViewMouseInputTest.java b/modules/controls/src/test/java/javafx/scene/control/ListViewMouseInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/ListViewMouseInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/ListViewMouseInputTest.java @@ -33,6 +33,7 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; +import javafx.collections.ListChangeListener; import org.junit.After; import org.junit.Before; import org.junit.Ignore; @@ -268,4 +269,33 @@ assertEquals(0, fm.getFocusedIndex()); assertEquals(1, sm.getSelectedItems().size()); } + + private int rt_30626_count = 0; + @Test public void test_rt_30626() { + final int items = 8; + listView.getItems().clear(); + for (int i = 0; i < items; i++) { + listView.getItems().add("Row " + i); + } + + final MultipleSelectionModel sm = listView.getSelectionModel(); + sm.setSelectionMode(SelectionMode.MULTIPLE); + sm.clearAndSelect(0); + + listView.getSelectionModel().getSelectedItems().addListener(new ListChangeListener() { + @Override public void onChanged(Change c) { + while (c.next()) { + System.out.println(c); + rt_30626_count++; + } + } + }); + + assertEquals(0, rt_30626_count); + VirtualFlowTestUtils.clickOnRow(listView, 1); + assertEquals(1, rt_30626_count); + + VirtualFlowTestUtils.clickOnRow(listView, 1); + assertEquals(1, rt_30626_count); + } } diff --git a/modules/controls/src/test/java/javafx/scene/control/TableViewKeyInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TableViewKeyInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TableViewKeyInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TableViewKeyInputTest.java @@ -84,8 +84,6 @@ col4 = new TableColumn("col4"); tableView.getColumns().setAll(col0, col1, col2, col3, col4); - sm.clearAndSelect(0); - keyboard = new KeyEventFirer(tableView); stageLoader = new StageLoader(tableView); @@ -159,9 +157,9 @@ **************************************************************************/ @Test public void testInitialState() { - assertTrue(sm.isSelected(0)); - assertEquals(1, sm.getSelectedIndices().size()); - assertEquals(1, sm.getSelectedItems().size()); + assertTrue(sm.getSelectedCells().isEmpty()); + assertTrue(sm.getSelectedIndices().isEmpty()); + assertTrue(sm.getSelectedItems().isEmpty()); } @@ -187,7 +185,10 @@ @Test public void testUpArrowDoesNotChangeSelectionWhenAt0Index() { sm.clearAndSelect(0); keyboard.doUpArrowPress(); - testInitialState(); + + assertTrue(sm.isSelected(0)); + assertEquals(1, sm.getSelectedIndices().size()); + assertEquals(1, sm.getSelectedItems().size()); } @Test public void testUpArrowChangesSelection() { @@ -218,6 +219,7 @@ // test 20 @Test public void testCtrlUpDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doUpArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -226,6 +228,7 @@ // test 21 @Test public void testCtrlLeftDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doLeftArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -234,6 +237,7 @@ // test 22 @Test public void testCtrlRightDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doRightArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); diff --git a/modules/controls/src/test/java/javafx/scene/control/TableViewMouseInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TableViewMouseInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TableViewMouseInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TableViewMouseInputTest.java @@ -34,6 +34,7 @@ import com.sun.javafx.tk.Toolkit; import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; +import javafx.collections.ListChangeListener; import javafx.scene.layout.StackPane; import org.junit.After; import org.junit.Before; @@ -382,4 +383,33 @@ assertEquals(0, fm.getFocusedIndex()); assertEquals(1, sm.getSelectedItems().size()); } + + private int rt_30626_count = 0; + @Test public void test_rt_30626() { + final int items = 8; + tableView.getItems().clear(); + for (int i = 0; i < items; i++) { + tableView.getItems().add("Row " + i); + } + + final MultipleSelectionModel sm = tableView.getSelectionModel(); + sm.setSelectionMode(SelectionMode.MULTIPLE); + sm.clearAndSelect(0); + + tableView.getSelectionModel().getSelectedItems().addListener(new ListChangeListener() { + @Override public void onChanged(Change c) { + while (c.next()) { + System.out.println(c); + rt_30626_count++; + } + } + }); + + assertEquals(0, rt_30626_count); + VirtualFlowTestUtils.clickOnRow(tableView, 1); + assertEquals(1, rt_30626_count); + + VirtualFlowTestUtils.clickOnRow(tableView, 1); + assertEquals(1, rt_30626_count); + } } diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewKeyInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewKeyInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewKeyInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewKeyInputTest.java @@ -125,8 +125,6 @@ col4 = new TreeTableColumn("col4"); tableView.getColumns().setAll(col0, col1, col2, col3, col4); - sm.clearAndSelect(0); - keyboard = new KeyEventFirer(tableView); stageLoader = new StageLoader(tableView); @@ -219,9 +217,9 @@ **************************************************************************/ @Test public void testInitialState() { - assertTrue(sm.isSelected(0)); - assertEquals(1, sm.getSelectedIndices().size()); - assertEquals(1, sm.getSelectedItems().size()); + assertTrue(sm.getSelectedCells().isEmpty()); + assertTrue(sm.getSelectedIndices().isEmpty()); + assertTrue(sm.getSelectedItems().isEmpty()); } @@ -247,7 +245,10 @@ @Test public void testUpArrowDoesNotChangeSelectionWhenAt0Index() { sm.clearAndSelect(0); keyboard.doUpArrowPress(); - testInitialState(); + assertTrue(sm.isSelected(0)); + assertEquals(1, sm.getSelectedCells().size()); + assertEquals(1, sm.getSelectedIndices().size()); + assertEquals(1, sm.getSelectedItems().size()); } @Test public void testUpArrowChangesSelection() { @@ -278,6 +279,7 @@ // test 20 @Test public void testCtrlUpDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doUpArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -286,6 +288,7 @@ // test 21 @Test public void testCtrlLeftDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doLeftArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -294,6 +297,7 @@ // test 22 @Test public void testCtrlRightDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doRightArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -1705,6 +1709,7 @@ // test 19 @Test public void testCtrlDownMovesFocusButLeavesSelectionAlone() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doDownArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(1)); diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java @@ -39,6 +39,7 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import javafx.scene.Group; import javafx.scene.Node; @@ -135,8 +136,6 @@ tableView.setRoot(root); tableView.getColumns().setAll(col0, col1, col2, col3, col4); - sm.clearAndSelect(0); - stageLoader = new StageLoader(tableView); stageLoader.getStage().show(); } @@ -520,4 +519,36 @@ assertTrue(root.isExpanded()); assertEquals(9, tableView.getExpandedItemCount()); } + + private int rt_30626_count = 0; + @Test public void test_rt_30626() { + final int items = 8; + root.getChildren().clear(); + root.setExpanded(true); + for (int i = 0; i < items; i++) { + root.getChildren().add(new TreeItem<>("Row " + i)); + } + tableView.setRoot(root); + + tableView.setShowRoot(true); + final MultipleSelectionModel sm = tableView.getSelectionModel(); + sm.setSelectionMode(SelectionMode.MULTIPLE); + sm.clearAndSelect(0); + + tableView.getSelectionModel().getSelectedItems().addListener(new ListChangeListener() { + @Override public void onChanged(Change c) { + while (c.next()) { + System.out.println(c); + rt_30626_count++; + } + } + }); + + assertEquals(0, rt_30626_count); + VirtualFlowTestUtils.clickOnRow(tableView, 1); + assertEquals(1, rt_30626_count); + + VirtualFlowTestUtils.clickOnRow(tableView, 1); + assertEquals(1, rt_30626_count); + } } diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeViewKeyInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeViewKeyInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeViewKeyInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewKeyInputTest.java @@ -106,7 +106,6 @@ treeView.setRoot(root); sm = treeView.getSelectionModel(); sm.setSelectionMode(SelectionMode.MULTIPLE); - sm.clearAndSelect(0); fm = treeView.getFocusModel(); // set up keyboard event firer @@ -175,9 +174,8 @@ **************************************************************************/ @Test public void testInitialState() { - assertTrue(sm.isSelected(0)); - assertEquals(1, sm.getSelectedIndices().size()); - assertEquals(1, sm.getSelectedItems().size()); + assertTrue(sm.getSelectedIndices().isEmpty()); + assertTrue(sm.getSelectedItems().isEmpty()); } /*************************************************************************** @@ -202,7 +200,9 @@ @Test public void testUpArrowDoesNotChangeSelectionWhenAt0Index() { sm.clearAndSelect(0); keyboard.doUpArrowPress(); - testInitialState(); + assertTrue(sm.isSelected(0)); + assertEquals(1, sm.getSelectedIndices().size()); + assertEquals(1, sm.getSelectedItems().size()); } @Test public void testUpArrowChangesSelection() { @@ -213,12 +213,16 @@ } @Test public void testLeftArrowDoesNotChangeState() { + sm.clearAndSelect(0); keyboard.doLeftArrowPress(); - testInitialState(); + assertTrue(sm.isSelected(0)); + assertEquals(1, sm.getSelectedIndices().size()); + assertEquals(1, sm.getSelectedItems().size()); } // test 19 @Test public void testCtrlDownMovesFocusButLeavesSelectionAlone() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doDownArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(1)); @@ -228,6 +232,7 @@ // test 20 @Test public void testCtrlUpDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doUpArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -236,6 +241,7 @@ // test 21 @Test public void testCtrlLeftDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doLeftArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); @@ -244,6 +250,7 @@ // test 22 @Test public void testCtrlRightDoesNotMoveFocus() { + sm.clearAndSelect(0); assertTrue(fm.isFocused(0)); keyboard.doRightArrowPress(KeyModifier.getShortcutKey()); assertTrue(fm.isFocused(0)); diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java @@ -34,6 +34,7 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; +import javafx.collections.ListChangeListener; import javafx.scene.Group; import javafx.scene.Scene; import javafx.scene.input.KeyCode; @@ -122,7 +123,6 @@ treeView.setRoot(root); sm = treeView.getSelectionModel(); sm.setSelectionMode(SelectionMode.MULTIPLE); - sm.clearAndSelect(0); fm = treeView.getFocusModel(); // set up keyboard event firer @@ -367,4 +367,36 @@ assertTrue("Selected items: " + sm.getSelectedItems(), sm.getSelectedItems().contains(root)); assertEquals(6, sm.getSelectedItems().size()); } + + private int rt_30626_count = 0; + @Test public void test_rt_30626() { + final int items = 8; + root.getChildren().clear(); + root.setExpanded(true); + for (int i = 0; i < items; i++) { + root.getChildren().add(new TreeItem<>("Row " + i)); + } + treeView.setRoot(root); + + treeView.setShowRoot(true); + final MultipleSelectionModel sm = treeView.getSelectionModel(); + sm.setSelectionMode(SelectionMode.MULTIPLE); + sm.clearAndSelect(0); + + treeView.getSelectionModel().getSelectedItems().addListener(new ListChangeListener() { + @Override public void onChanged(Change c) { + while (c.next()) { + System.out.println(c); + rt_30626_count++; + } + } + }); + + assertEquals(0, rt_30626_count); + VirtualFlowTestUtils.clickOnRow(treeView, 1); + assertEquals(1, rt_30626_count); + + VirtualFlowTestUtils.clickOnRow(treeView, 1); + assertEquals(1, rt_30626_count); + } }