8290348: TreeTableView jumping to top

Reviewed-by: aghaisas, kcr
openjdk · Jul 21, 2022 · 6da05c2 · 6da05c2 · openjdk-notifier · Jul 21, 2022
1 parent 25e8605
commit 6da05c2
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 35 deletions.
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
@@ -3069,45 +3069,51 @@ private void recalculateEstimatedSize() {
         recalculateAndImproveEstimatedSize(DEFAULT_IMPROVEMENT);
     }
 
+    private boolean recalculating = false;
+
     private void recalculateAndImproveEstimatedSize(int improve) {
-        int itemCount = getCellCount();
-        int cacheCount = itemSizeCache.size();
-        boolean keepRatio = ((cacheCount > 0) && !Double.isInfinite(this.absoluteOffset));
-        double estSize = estimatedSize / itemCount;
-
-        int oldIndex = computeCurrentIndex();
-        double oldOffset = computeViewportOffset(getPosition());
-        int added = 0;
-        while ((itemCount > itemSizeCache.size()) && (added < improve)) {
-            getOrCreateCellSize(itemSizeCache.size());
-            added++;
-        }
-        cacheCount = itemSizeCache.size();
-        int cnt = 0;
-        double tot = 0d;
-        for (int i = 0; (i < itemCount && i < cacheCount); i++) {
-            Double il = itemSizeCache.get(i);
-            if (il != null) {
-                tot = tot + il;
-                cnt++;
-            }
-        }
-        this.estimatedSize = cnt == 0 ? 1d : tot * itemCount / cnt;
-        estSize = estimatedSize / itemCount;
-
-        if (keepRatio) {
-            double newOffset = 0;
-            for (int i = 0; i < oldIndex; i++) {
-                double h = getCellSize(i);
-                if (h < 0) {
-                    h = estSize;
+        if (recalculating) return;
+        recalculating = true;
+        try {
+            int itemCount = getCellCount();
+            int cacheCount = itemSizeCache.size();
+            boolean keepRatio = ((cacheCount > 0) && !Double.isInfinite(this.absoluteOffset));
+
+            int oldIndex = computeCurrentIndex();
+            double oldOffset = computeViewportOffset(getPosition());
+            int added = 0;
+            while ((itemCount > itemSizeCache.size()) && (added < improve)) {
+                getOrCreateCellSize(itemSizeCache.size());
+                added++;
+            }
+            cacheCount = itemSizeCache.size();
+            int cnt = 0;
+            double tot = 0d;
+            for (int i = 0; (i < itemCount && i < cacheCount); i++) {
+                Double il = itemSizeCache.get(i);
+                if (il != null) {
+                    tot = tot + il;
+                    cnt++;
                 }
-                newOffset += h;
             }
-            this.absoluteOffset = newOffset + oldOffset;
-            adjustPosition();
-        }
+            this.estimatedSize = cnt == 0 ? 1d : tot * itemCount / cnt;
+            double estSize = estimatedSize / itemCount;
 
+            if (keepRatio) {
+                double newOffset = 0;
+                for (int i = 0; i < oldIndex; i++) {
+                    double h = getCellSize(i);
+                    if (h < 0) {
+                        h = estSize;
+                    }
+                    newOffset += h;
+                }
+                this.absoluteOffset = newOffset + oldOffset;
+                adjustPosition();
+            }
+        } finally {
+            recalculating = false;
+        }
     }
 
     private void resetSizeEstimates() {

diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java
@@ -3907,6 +3907,35 @@ public void testRemoveTreeItemFromCollapsedAncestorKeepsSelectedItem() {
         assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
     }
 
+    private List<TreeItem<String>> generateChildren(int lvl) {
+        List<TreeItem<String>> children = new ArrayList<>();
+        for (int idx = 0; idx < 10; idx++) {
+            TreeItem<String> child = new TreeItem<>("Child lvl. " + lvl + " idx. " + idx);
+            child.setExpanded(true);
+            if (lvl <= 2) {
+                child.getChildren().addAll(generateChildren(lvl + 1));
+            }
+            children.add(child);
+        }
+        return children;
+    }
+
+    // JDK-8290348
+    @Test
+    public void testCheckPositionAfterCollapsed() {
+        TreeItem<String> rootNode = new TreeItem<>("Root");
+        rootNode.setExpanded(true);
+        rootNode.getChildren().addAll(generateChildren(1));
+        TreeView<String> treeView = new TreeView<>(rootNode);
+        treeView.scrollTo(100);
+        IndexedCell expandedCell = VirtualFlowTestUtils.getCell(treeView, 100);
+        Toolkit.getToolkit().firePulse();
+        rootNode.getChildren().get(1).setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        IndexedCell scrolledCell = VirtualFlowTestUtils.getCell(treeView, 100);
+        assertTrue(scrolledCell.isVisible());
+    }
+
     public static class MisbehavingOnCancelTreeCell<S> extends TreeCell<S> {
 
         @Override