8237602: TabPane doesn't respect order of TabPane.getTabs() list

Reviewed-by: kcr, fastegal
openjdk · May 18, 2020 · 6e03930 · 6e03930
1 parent bb24322
commit 6e03930
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 6 deletions.
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
@@ -579,7 +579,6 @@ private void initializeTabListener() {
         getSkinnable().getTabs().addListener((ListChangeListener<Tab>) c -> {
             List<Tab> tabsToRemove = new ArrayList<>();
             List<Tab> tabsToAdd = new ArrayList<>();
-            int insertPos = -1;
 
             while (c.next()) {
                 if (c.wasPermutated()) {
@@ -619,7 +618,6 @@ private void initializeTabListener() {
                 }
                 if (c.wasAdded()) {
                     tabsToAdd.addAll(c.getAddedSubList());
-                    insertPos = c.getFrom();
                 }
             }
 
@@ -647,7 +645,9 @@ private void initializeTabListener() {
                     }
                 }
 
-                addTabs(tabsToAdd, insertPos == -1 ? tabContentRegions.size() : insertPos);
+                if (!tabsToAdd.isEmpty()) {
+                    addTabs(tabsToAdd, getSkinnable().getTabs().indexOf(tabsToAdd.get(0)));
+                }
                 for (Pair<Integer, TabHeaderSkin> move : headersToMove) {
                     tabHeaderArea.moveTab(move.getKey(), move.getValue());
                 }
@@ -991,8 +991,10 @@ private void removeTab(Tab tab) {
         }
 
         private void moveTab(int moveToIndex, TabHeaderSkin tabHeaderSkin) {
-            headersRegion.getChildren().remove(tabHeaderSkin);
-            headersRegion.getChildren().add(moveToIndex, tabHeaderSkin);
+            if (moveToIndex != headersRegion.getChildren().indexOf(tabHeaderSkin)) {
+                headersRegion.getChildren().remove(tabHeaderSkin);
+                headersRegion.getChildren().add(moveToIndex, tabHeaderSkin);
+            }
         }
 
         private TabHeaderSkin getTabHeaderSkin(Tab tab) {

diff --git a/tests/system/src/test/java/test/robot/javafx/scene/TabPanePermuteGetTabsTest.java b/tests/system/src/test/java/test/robot/javafx/scene/TabPanePermuteGetTabsTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020, 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
@@ -70,6 +70,14 @@
  *       3.2 tab[1] is the first tab in tab header.
  *       3.3 Pressing RIGHT key should select tabs in order: tab 4,3,2,0
  *
+ * b1. testPermuteGetTabsWithMoreTabs1()
+ *    1. Add tabs 0,1
+ *    2. Permute tabs to tab 0,1,2,3 using TabPane.getTabs().setAll().
+ *    3. Verify that,
+ *       3.1 tab[1] should remain selected tab.
+ *       3.2 tab[0] is the first tab in tab header.
+ *       3.3 Pressing RIGHT key should select tabs in order: tab 1,2,3
+ *
  * c. testPermuteGetTabsWithLessTabs()
  *    1. Add tab 3,1 and some(6) more tabs, and select tab 1.
  *    2. Permute tabs to, tab 1,4,3,2 using TabPane.getTabs().setAll().
@@ -181,6 +189,59 @@ public void testPermuteGetTabsWithMoreTabs() {
         }
     }
 
+    // Test for JDK-8237602
+    @Test
+    public void testAddingNewTabsWithExistingTabsAtSameIndex() {
+        // Step #1
+        Util.runAndWait(() -> {
+            tabPane.getTabs().setAll(tab[0], tab[1]);
+            tabPane.getSelectionModel().select(tab[1]);
+        });
+        delay();
+
+        Assert.assertSame("Sanity: tab[1] should be the selected tab.",
+            tab[1], tabPane.getSelectionModel().getSelectedItem());
+
+        // Step #2
+        Util.runAndWait(() -> {
+            tabPane.getTabs().setAll(tab[0], tab[1], tab[2], tab[3]);
+        });
+        delay();
+
+        // Step #3.1
+        Assert.assertSame("Sanity: tab[1] should remain selected tab after permuting.",
+            tab[1], tabPane.getSelectionModel().getSelectedItem());
+
+        // Step #3.2
+        // Click on first tab header
+        selectionLatch[0] = new CountDownLatch(1);
+        Util.runAndWait(() -> {
+            robot.mouseMove((int) (scene.getWindow().getX() + scene.getX() + firstTabdXY),
+                    (int) (scene.getWindow().getY() + scene.getY() + firstTabdXY));
+            robot.mousePress(MouseButton.PRIMARY);
+            robot.mouseRelease(MouseButton.PRIMARY);
+        });
+        delay();
+
+        waitForLatch(selectionLatch[0], 5,
+            "Timeout: Waiting for tab[" + 0 + "] to get selected.");
+        Assert.assertSame("tab[0] should be first tab after permuting.",
+            tab[0], tabPane.getSelectionModel().getSelectedItem());
+
+        // step #3.3
+        selectionLatch[1] = new CountDownLatch(1);
+        for (int i = 1; i <= 3; i++) {
+            Util.runAndWait(() -> {
+                robot.keyPress(KeyCode.RIGHT);
+                robot.keyRelease(KeyCode.RIGHT);
+            });
+            waitForLatch(selectionLatch[i], 5,
+                "Timeout: Waiting for tab[" + i + "] to get selected.");
+            Assert.assertSame("tab[" + i + "] should get selected on RIGHT key press.",
+                tab[i], tabPane.getSelectionModel().getSelectedItem());
+        }
+    }
+
     @Test
     public void testPermutGetTabsWithLessTabs() {
         // Step #1