From 53d2c12c4ef62443a3995b12a21e64f91517bdae Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Mon, 26 Jan 2026 16:22:40 +0100 Subject: [PATCH 1/4] Fix(workbench): StackRenderer updates correctly when part is moved Fixes Eclipse Bug 576186. The StackRenderer now correctly updates its UI when a part within the stack is moved. This was achieved by handling UIEvents.isMOVE events for MPartStack children in StackRenderer.java. CTabItems, which represent tabs, cannot be reordered directly and must be disposed of and recreated at the new index. The methods calcIndexFor and findItemForPart in StackRenderer were updated to accept a more generic MElementContainer parameter to resolve type compatibility issues with MPartStack. --- .../renderers/swt/StackRenderer.java | 66 ++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java index 929e574be64b..a9aa317d3d74 100644 --- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java +++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java @@ -395,6 +395,60 @@ void subscribeTopicChildrenChanged(@UIEventTopic(UIEvents.ElementContainer.TOPIC shouldTopRightAdjusted(event); } + @Inject + @Optional + void subscribeTopicChildrenMoved(@UIEventTopic(UIEvents.ElementContainer.TOPIC_CHILDREN) Event event) { + if (!UIEvents.isMOVE(event)) { + return; + } + // Ensure that this event is for a MPartStack + Object element = event.getProperty(UIEvents.EventTags.ELEMENT); + if (!(element instanceof MPartStack)) { + return; + } + + MPartStack stack = (MPartStack) element; + if (stack.getRenderer() != this) { + return; + } + + MUIElement movedElement = (MUIElement) event.getProperty(UIEvents.EventTags.NEW_VALUE); + + CTabFolder tabFolder = (CTabFolder) stack.getWidget(); + if (tabFolder == null || tabFolder.isDisposed()) { + return; + } + + CTabItem item = findItemForPart(movedElement, stack); + if (item == null || item.isDisposed()) { + return; + } + + int newIndex = calcIndexFor(stack, movedElement); + + // Remember the control, it will be disposed with the CTabItem otherwise + Control control = item.getControl(); + item.setControl(null); + + // As CTabItem cannot be reordered, we need to dispose and recreate it + String text = item.getText(); + Image image = item.getImage(); + boolean showClose = item.getShowClose(); + String toolTipText = item.getToolTipText(); + Font font = item.getFont(); + Object data = item.getData(); + + item.dispose(); + + CTabItem newItem = new CTabItem(tabFolder, (showClose ? SWT.CLOSE : SWT.NONE), newIndex); + newItem.setText(text); + newItem.setImage(image); + newItem.setToolTipText(toolTipText); + newItem.setFont(font); + newItem.setData(data); + newItem.setControl(control); + } + @Inject @Optional void subscribeTopicUILabelChanged(@UIEventTopic(UIEvents.UILabel.TOPIC_ALL) Event event) { @@ -623,6 +677,14 @@ void subscribeTopicSelectedelementChanged( tabStateHandler.handleEvent(event); } + @Override + public void removeGui(MUIElement element, Object widget) { + if (widget instanceof CTabFolder tabFolder && !tabFolder.isDisposed()) { + tabFolder.dispose(); + } + element.setWidget(null); + } + @Override protected boolean requiresFocus(MPart element) { MUIElement inStack = element.getCurSharedRef() != null ? element.getCurSharedRef() : element; @@ -1010,7 +1072,7 @@ protected void createTab(MElementContainer stack, MUIElement element } } - private int calcIndexFor(MElementContainer stack, final MUIElement part) { + private int calcIndexFor(MElementContainer stack, final MUIElement part) { int index = 0; // Find the -visible- part before this element @@ -1036,7 +1098,7 @@ public void childRendered(final MElementContainer parentElement, MUI createTab(parentElement, element); } - private CTabItem findItemForPart(MUIElement element, MElementContainer stack) { + private CTabItem findItemForPart(MUIElement element, MElementContainer stack) { if (stack == null) { stack = element.getParent(); } From 9351f65164af083d06ae67bb454f9796ad3c3bfb Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 27 Jan 2026 10:27:50 +0100 Subject: [PATCH 2/4] Add test case for StackRenderer part reordering --- .../renderers/swt/StackRendererTest.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java index f5e9564a89ef..bcd45aab57c1 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java @@ -573,6 +573,32 @@ public void testToolbarIsReparentedToNewCompositeForTopRightOfTabFolder() { assertSame(tabFolder.getTopRight(), toolbarControl.getParent()); } + @Test + public void testPartReordering() { + MPart part1 = ems.createModelElement(MPart.class); + part1.setLabel("Part 1"); + MPart part2 = ems.createModelElement(MPart.class); + part2.setLabel("Part 2"); + + partStack.getChildren().add(part1); + partStack.getChildren().add(part2); + + contextRule.createAndRunWorkbench(window); + + CTabFolder tabFolder = (CTabFolder) partStack.getWidget(); + assertEquals(2, tabFolder.getItemCount()); + assertEquals("Part 1", tabFolder.getItem(0).getText()); + assertEquals("Part 2", tabFolder.getItem(1).getText()); + + // Move part2 to index 0 + partStack.getChildren().remove(part2); + partStack.getChildren().add(0, part2); + + // Verify order in Widget + assertEquals("Part 2", tabFolder.getItem(0).getText()); + assertEquals("Part 1", tabFolder.getItem(1).getText()); + } + // helper functions /* From c4502022615fad160853ae88d3a0fdf2dc33dcc5 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 27 Jan 2026 20:33:00 +0100 Subject: [PATCH 3/4] Fix(workbench): StackRenderer duplication of tabs on move Ensure OWNING_ME data is preserved when a CTabItem is recreated during a move operation. This prevents the renderer from creating duplicate tabs for the same part. --- .../renderers/swt/StackRenderer.java | 1 + .../renderers/swt/StackRendererMoveTest.java | 97 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java index a9aa317d3d74..c6a887a75900 100644 --- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java +++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java @@ -446,6 +446,7 @@ void subscribeTopicChildrenMoved(@UIEventTopic(UIEvents.ElementContainer.TOPIC_C newItem.setToolTipText(toolTipText); newItem.setFont(font); newItem.setData(data); + newItem.setData(OWNING_ME, movedElement); newItem.setControl(control); } diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java new file mode 100644 index 000000000000..02371bcbb1a6 --- /dev/null +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java @@ -0,0 +1,97 @@ +package org.eclipse.e4.ui.workbench.renderers.swt; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.inject.Inject; +import java.util.List; +import org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer; +import org.eclipse.e4.ui.model.application.MApplication; +import org.eclipse.e4.ui.model.application.ui.basic.MPart; +import org.eclipse.e4.ui.model.application.ui.basic.MPartStack; +import org.eclipse.e4.ui.model.application.ui.basic.MWindow; +import org.eclipse.e4.ui.tests.rules.WorkbenchContextExtension; +import org.eclipse.e4.ui.workbench.modeling.EModelService; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class StackRendererMoveTest { + + @RegisterExtension + public WorkbenchContextExtension contextRule = new WorkbenchContextExtension(); + + @Inject + private EModelService ems; + + @Inject + private MApplication application; + + private MWindow window; + private MPartStack partStack; + + @BeforeEach + public void setUp() throws Exception { + window = ems.createModelElement(MWindow.class); + application.getChildren().add(window); + application.setSelectedElement(window); + + partStack = ems.createModelElement(MPartStack.class); + window.getChildren().add(partStack); + } + + @Test + public void testPartMoveUpdatesWidget() throws Exception { + // Create two parts + MPart part1 = ems.createModelElement(MPart.class); + part1.setLabel("Part 1"); + partStack.getChildren().add(part1); + + MPart part2 = ems.createModelElement(MPart.class); + part2.setLabel("Part 2"); + partStack.getChildren().add(part2); + + // Render the window (and thus the stack and parts) + contextRule.createAndRunWorkbench(window); + + CTabFolder tabFolder = (CTabFolder) partStack.getWidget(); + assertEquals(2, tabFolder.getItemCount()); + + CTabItem item1 = tabFolder.getItem(0); + CTabItem item2 = tabFolder.getItem(1); + + assertEquals(part1, item1.getData(AbstractPartRenderer.OWNING_ME)); + assertEquals(part2, item2.getData(AbstractPartRenderer.OWNING_ME)); + assertEquals(item1.getControl(), part1.getWidget()); + assertEquals(item2.getControl(), part2.getWidget()); + + // Move part1 to the end (index 1) + // We use model service to move to ensure events are fired + ems.move(part1, partStack, 1); + + // Verify model update + List children = partStack.getChildren(); + assertEquals(part2, children.get(0)); + assertEquals(part1, children.get(1)); + + // Verify UI update + assertEquals(2, tabFolder.getItemCount()); + CTabItem newItem1 = tabFolder.getItem(1); + CTabItem newItem2 = tabFolder.getItem(0); + + // The old item1 should be disposed + assertTrue(item1.isDisposed(), "Old item for part1 should be disposed"); + + // part1 should have a NEW widget item, but the part's widget (content) should be preserved + assertNotSame(item1, newItem1); + assertEquals(part1, newItem1.getData(AbstractPartRenderer.OWNING_ME), "New item should have OWNING_ME set"); + assertEquals(part1.getWidget(), newItem1.getControl(), "Part1 widget should be the control of the new item"); + + // part2 should still be valid and same widget + assertEquals(item2, newItem2); + assertEquals(part2, newItem2.getData(AbstractPartRenderer.OWNING_ME)); + } +} From 9a9b1022eb175c67db108000226e81d28b895cfa Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Wed, 28 Jan 2026 17:25:01 +0100 Subject: [PATCH 4/4] Fix tab selection on move and enhance test coverage --- .../eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java | 5 +++++ .../e4/ui/workbench/renderers/swt/StackRendererMoveTest.java | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java index c6a887a75900..1ec84b00b5d3 100644 --- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java +++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java @@ -438,6 +438,8 @@ void subscribeTopicChildrenMoved(@UIEventTopic(UIEvents.ElementContainer.TOPIC_C Font font = item.getFont(); Object data = item.getData(); + boolean wasSelected = tabFolder.getSelection() == item; + item.dispose(); CTabItem newItem = new CTabItem(tabFolder, (showClose ? SWT.CLOSE : SWT.NONE), newIndex); @@ -448,6 +450,9 @@ void subscribeTopicChildrenMoved(@UIEventTopic(UIEvents.ElementContainer.TOPIC_C newItem.setData(data); newItem.setData(OWNING_ME, movedElement); newItem.setControl(control); + if (wasSelected) { + tabFolder.setSelection(newItem); + } } @Inject diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java index 02371bcbb1a6..38e39a509d9e 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererMoveTest.java @@ -1,6 +1,7 @@ package org.eclipse.e4.ui.workbench.renderers.swt; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -84,7 +85,8 @@ public void testPartMoveUpdatesWidget() throws Exception { // The old item1 should be disposed assertTrue(item1.isDisposed(), "Old item for part1 should be disposed"); - + assertFalse(item2.isDisposed(), "Item2 should not be disposed"); + // part1 should have a NEW widget item, but the part's widget (content) should be preserved assertNotSame(item1, newItem1); assertEquals(part1, newItem1.getData(AbstractPartRenderer.OWNING_ME), "New item should have OWNING_ME set");