From 562ad674d4c98981c1668d1ae39c9944e9f0cc89 Mon Sep 17 00:00:00 2001 From: "Patrick.Ziegler" Date: Mon, 10 Nov 2025 14:08:38 +0100 Subject: [PATCH] Don't create dummy item when removing invalid element from tree #3525 Due to the fix for Bug 210747, a dummy tree-item is added to the viewer if the same element is removed twice. The problem is that this fix doesn't distinguish between nodes that haven't been realized yet and nodes that are not part of the model. This check has been refined to only call updatePlus(...) if the removed object is associated with such a dummy node. To reproduce: - Create a new project "Project A" and "Project B" - Create a new text file "test" in "Project A" - Move "test" to "Project B" Closes https://github.com/eclipse-platform/eclipse.platform.ui/issues/3525 --- .../jface/viewers/AbstractTreeViewer.java | 9 +-- .../jface/tests/viewers/TreeViewerTest.java | 74 ++++++++++++++++++- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java index 3b02bf424fa..954d851b421 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2019 IBM Corporation and others. + * Copyright (c) 2000, 2025 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -2095,12 +2095,7 @@ protected void internalRemove(Object[] elementsOrPaths) { Object parent = getParentElement(element); if (parent != null && !equals(parent, getRoot()) && !(parent instanceof TreePath tp && tp.getSegmentCount() == 0)) { - Widget[] parentItems = internalFindItems(parent); - for (Widget parentItem : parentItems) { - if (parentItem instanceof Item it) { - updatePlus(it, parent); - } - } + internalRemove(parent, new Object[] { element }); } } } diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/TreeViewerTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/TreeViewerTest.java index 4d325a0202e..7198c8937fb 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/TreeViewerTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/TreeViewerTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2006 IBM Corporation and others. + * Copyright (c) 2000, 2025 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -14,7 +14,11 @@ package org.eclipse.jface.tests.viewers; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.util.ArrayList; import java.util.Arrays; @@ -31,6 +35,7 @@ import org.eclipse.swt.widgets.Tree; import org.eclipse.swt.widgets.TreeItem; import org.eclipse.swt.widgets.Widget; +import org.junit.Ignore; import org.junit.Test; public class TreeViewerTest extends AbstractTreeViewerTest { @@ -114,4 +119,71 @@ protected void internalExpandToLevel(Widget widget, int level) { } } + /** + * Removing the same element twice should not produce a dummy tree-item. + */ + @Test + public void testIssue3525() { + TestElement modelRoot = TestElement.createModel(2, 1); + TestElement modelParent = modelRoot.getChildAt(0); + TestElement modelChild = modelParent.getChildAt(0); + fTreeViewer.setInput(modelRoot); + fTreeViewer.expandAll(); + + TreeItem widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent); + TreeItem widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild); + assertNotNull(widgetParent); + assertNotNull(widgetChild); + assertArrayEquals(widgetParent.getItems(), new TreeItem[] { widgetChild }); + + // This workaround is needed because of TreeViewerWithLimitCompatibilityTest + // When calling setDisplayIncrementally(...) with a positive number, you are + // no longer able to remove elements from the viewer without first removing + // them from the model + modelParent.fChildren.remove(modelChild); + fTreeViewer.remove(modelChild); + modelParent.fChildren.add(modelChild); + + widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent); + widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild); + assertNotNull(widgetParent); + assertNull(widgetChild); + assertArrayEquals(widgetParent.getItems(), new TreeItem[0]); + + fTreeViewer.remove(modelChild); + + widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent); + widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild); + assertNotNull(widgetParent); + assertNull(widgetChild); + assertArrayEquals(widgetParent.getItems(), new TreeItem[0]); + } + + /** + * Remove an element that hasn't been realized yet. + */ + @Test + @Ignore + public void testBug210747() { + TestElement modelRoot = TestElement.createModel(2, 1); + TestElement modelParent = modelRoot.getChildAt(0); + TestElement modelChild = modelParent.getChildAt(0); + fTreeViewer.setInput(modelRoot); + fTreeViewer.setExpandedElements(modelRoot); + + TreeItem widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent); + TreeItem widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild); + assertNotNull(widgetParent); + assertNull(widgetChild); + assertEquals(widgetParent.getItems().length, 1); // Dummy Item + assertNull(widgetParent.getItems()[0].getData()); + + fTreeViewer.remove(modelChild); + + widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent); + widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild); + assertNotNull(widgetParent); + assertNull(widgetChild); + assertArrayEquals(widgetParent.getItems(), new TreeItem[0]); + } }