From acce1d719e62e2baf943074969fb96be86f28019 Mon Sep 17 00:00:00 2001 From: insjang Date: Wed, 30 Sep 2026 10:29:43 +0900 Subject: [PATCH 1/4] Terminal: fix moving a terminal tab to another terminals view Dragging a terminal tab to another terminals view recreates the terminal controls in the new view. This failed in two ways: - The text line renderer of the new canvas measured the font on fCtlText, which at that point still is the canvas that was just disposed. The move stopped with "Widget is disposed", leaving the tab behind and both terminals blank. - The preferences were only applied to freshly created terminals, so a moved terminal fell back to the default colors and font. The renderer now measures on the display while the new canvas does not exist yet, and the preferences are applied to recreated controls too. --- .../emulator/VT100TerminalControl.java | 12 ++- .../TerminalControlRecreateUITest.java | 92 +++++++++++++++++++ .../terminal/test/AutomatedTestSuite.java | 1 + 3 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java diff --git a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100TerminalControl.java b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100TerminalControl.java index e80d897f11c..8fa24651171 100644 --- a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100TerminalControl.java +++ b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100TerminalControl.java @@ -657,9 +657,12 @@ public void setupTerminal(Composite parent) { setupControls(parent); setCommandInputField(fCommandInputField); setupListeners(); - if (fPreferenceStore != null && wasDisposed) { + if (fPreferenceStore != null) { + // new controls after a drag and drop need the colors and font as well updatePreferences(null); - fPreferenceStore.addPropertyChangeListener(fPreferenceListener); + if (wasDisposed) { + fPreferenceStore.addPropertyChangeListener(fPreferenceListener); + } } JFaceResources.getFontRegistry().addListener(fFontListener); setupHelp(fWndParent, TerminalPlugin.HELP_VIEW); @@ -763,7 +766,10 @@ protected void setupControls(Composite parent) { snapshot.updateSnapshot(false); fPollingTextCanvasModel = new PollingTextCanvasModel(snapshot); fCtlText = new TextCanvas(fWndParent, fPollingTextCanvasModel, SWT.NONE, - new TextLineRenderer(() -> fCtlText, fPollingTextCanvasModel)); + // While the canvas is being created, fCtlText may still be the canvas disposed by a + // drag and drop to another tab folder: measure on the display then + new TextLineRenderer(() -> fCtlText != null && !fCtlText.isDisposed() ? fCtlText : null, + fPollingTextCanvasModel)); fCtlText.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); fCtlText.addResizeHandler((lines, columns) -> fTerminalText.setDimensions(lines, columns)); diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java new file mode 100644 index 00000000000..df09d40590e --- /dev/null +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java @@ -0,0 +1,92 @@ +/******************************************************************************* + * Copyright (c) 2026 Eclipse contributors and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.terminal.internal.textcanvas; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.eclipse.jface.preference.PreferenceStore; +import org.eclipse.swt.graphics.RGB; +import org.eclipse.swt.layout.FillLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.terminal.connector.ITerminalConnector; +import org.eclipse.terminal.connector.TerminalState; +import org.eclipse.terminal.control.ITerminalListener; +import org.eclipse.terminal.control.TerminalTitleRequestor; +import org.eclipse.terminal.internal.emulator.VT100TerminalControl; +import org.eclipse.terminal.internal.preferences.ITerminalConstants; +import org.eclipse.terminal.model.TerminalColor; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Tests recreating the terminal controls in a new parent, as moving a terminal tab to another + * terminals view does. + */ +public class TerminalControlRecreateUITest { + + private static final RGB BACKGROUND = new RGB(1, 2, 3); + + private Display display; + private Shell shell; + private VT100TerminalControl terminal; + + @BeforeEach + public void createTerminal() { + display = Display.getCurrent() != null ? null : new Display(); + shell = new Shell(); + shell.setLayout(new FillLayout()); + PreferenceStore preferences = new PreferenceStore(); + preferences.setValue(ITerminalConstants.getPrefForTerminalColor(TerminalColor.BACKGROUND), + "1,2,3"); //$NON-NLS-1$ + ITerminalListener listener = new ITerminalListener() { + @Override + public void setState(TerminalState state) { + } + + @Override + public void setTerminalSelectionChanged() { + } + + @Override + public void setTerminalTitle(String title, TerminalTitleRequestor requestor) { + } + }; + terminal = new VT100TerminalControl(listener, new Composite(shell, 0), new ITerminalConnector[0], preferences); + } + + @AfterEach + public void dispose() { + shell.dispose(); + if (display != null) { + display.dispose(); + } + } + + private void recreateInNewParent() { + terminal.setupTerminal(new Composite(shell, 0)); + } + + @Test + public void recreatingDoesNotUseTheDisposedCanvas() { + assertDoesNotThrow(this::recreateInNewParent); + } + + @Test + public void recreatedCanvasKeepsThePreferences() { + assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getTerminalBackgroundColor(shell.getDisplay()).getRGB()); + recreateInNewParent(); + assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getTerminalBackgroundColor(shell.getDisplay()).getRGB()); + } +} diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/test/AutomatedTestSuite.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/test/AutomatedTestSuite.java index c146029c61a..05bd86f43c3 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/test/AutomatedTestSuite.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/test/AutomatedTestSuite.java @@ -25,6 +25,7 @@ org.eclipse.terminal.model.AllTestSuite.class, // org.eclipse.terminal.internal.connector.TerminalConnectorTest.class, // org.eclipse.terminal.internal.connector.TerminalToRemoteInjectionOutputStreamTest.class, // + org.eclipse.terminal.internal.textcanvas.TerminalControlRecreateUITest.class, // org.eclipse.terminal.view.ui.tests.TerminalsViewReorderTest.class, // }) public class AutomatedTestSuite { From 366fd25b6881b2d4a97fba102dd2daa02ecb99db Mon Sep 17 00:00:00 2001 From: Eclipse Platform Bot Date: Wed, 30 Sep 2026 08:20:56 +0000 Subject: [PATCH 2/4] Version bump(s) for 4.42 stream --- .../bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF b/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF index ff5a3fb72be..0c618bbd154 100644 --- a/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF +++ b/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.terminal.control; singleton:=true -Bundle-Version: 1.1.200.qualifier +Bundle-Version: 1.1.300.qualifier Bundle-Activator: org.eclipse.terminal.internal.control.impl.TerminalPlugin Bundle-Vendor: %providerName Bundle-Localization: plugin From f0b681210924300008b8e45beb6169021b229f65 Mon Sep 17 00:00:00 2001 From: insjang Date: Wed, 30 Sep 2026 18:33:58 +0900 Subject: [PATCH 3/4] Terminal: dispose the terminal after the recreate test As the terminals view does when a terminal tab is closed, so the terminal does not stay registered in the font registry after the test. --- .../internal/textcanvas/TerminalControlRecreateUITest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java index df09d40590e..119506c6a86 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java @@ -68,6 +68,8 @@ public void setTerminalTitle(String title, TerminalTitleRequestor requestor) { @AfterEach public void dispose() { + // as the terminals view does when a terminal tab is closed + terminal.disposeTerminal(); shell.dispose(); if (display != null) { display.dispose(); From d17d8a48e11052947d502785b17ebce0cac5e442 Mon Sep 17 00:00:00 2001 From: insjang Date: Wed, 30 Sep 2026 19:15:24 +0900 Subject: [PATCH 4/4] Terminal: read the background color through the renderer in the test TextCanvas.getTerminalBackgroundColor() is protected: in OSGi the test bundle is another runtime package, so the test failed with an IllegalAccessError. The renderer's getDefaultBackgroundColor() is public. --- .../internal/textcanvas/TerminalControlRecreateUITest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java index 119506c6a86..f21ca7433eb 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/textcanvas/TerminalControlRecreateUITest.java @@ -87,8 +87,8 @@ public void recreatingDoesNotUseTheDisposedCanvas() { @Test public void recreatedCanvasKeepsThePreferences() { - assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getTerminalBackgroundColor(shell.getDisplay()).getRGB()); + assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getCellRenderer().getDefaultBackgroundColor().getRGB()); recreateInNewParent(); - assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getTerminalBackgroundColor(shell.getDisplay()).getRGB()); + assertEquals(BACKGROUND, ((TextCanvas) terminal.getControl()).getCellRenderer().getDefaultBackgroundColor().getRGB()); } }