From 379a78222fc695ed442d1fdc6faa8783db3f6a7c Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 14 Aug 2026 13:06:34 +0200 Subject: [PATCH] Do not switch to the unified diff after the comparison was closed The switch to the unified diff reads its source in a job and opens the editor afterwards. It looked up the compare editor only to decide whether to close it, so a comparison the user closed in the meantime still brought up a unified diff for an editor that was already gone. The switch now stops when the compare editor can no longer be found, which also covers a page that is closing. The toolbar action test compared the label against the English text. A missing resource key makes Utilities.initAction fall back to the key name, so the assertion is against that instead, which keeps the missing key detected without breaking under a translated label. --- .../compare/internal/CompareUIPlugin.java | 6 +++- .../compare/tests/UnifiedDiffOpenTest.java | 28 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java index f3df9ab744a..52900bd8f2e 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java @@ -957,7 +957,11 @@ protected IStatus run(IProgressMonitor monitor) { } Display.getDefault().asyncExec(() -> { IEditorPart compareEditor = wpage.findEditor(input); - if (openUnifiedDiff(source, input, wpage, null, true) && compareEditor != null) { + if (compareEditor == null) { + // The comparison was closed while it was being prepared. + return; + } + if (openUnifiedDiff(source, input, wpage, null, true)) { // Prompts when the merge has unsaved changes. wpage.closeEditor(compareEditor, true); } diff --git a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java index 95329468bd1..688cfb97d93 100644 --- a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java +++ b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java @@ -58,6 +58,7 @@ import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jface.action.Action; import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.resource.ImageDescriptor; @@ -621,7 +622,27 @@ public void testSwitchToUnifiedDiffReplacesTheCompareEditor() throws Exception { "switching must reuse the already prepared input"); //$NON-NLS-1$ } - /** A missing resource key would leave the toolbar button blank. */ + /** + * A comparison the user closed while the switch was being prepared must not + * bring up an editor afterwards. + */ + @Test + public void testSwitchToUnifiedDiffStopsWhenTheComparisonIsClosed() throws Exception { + RecordingCompareEditorInput input = openClassicInput(); + + CompareUIPlugin.getDefault().switchToUnifiedDiff(input, activePage()); + // Still on the display thread, so this lands before the switch can open + // anything. + activePage().closeEditor(activePage().findEditor(input), false); + + pumpUntil(() -> Job.getJobManager().find(input).length == 0, "the switch job did not finish"); //$NON-NLS-1$ + processQueuedEvents(); + + assertEquals(0, activePage().getEditorReferences().length, + "no editor may open once the comparison is closed"); //$NON-NLS-1$ + } + + /** A missing resource key would label the toolbar button with the key itself. */ @Test public void testShowUnifiedDiffActionIsFullyDescribed() { ResourceBundle bundle = ResourceBundle.getBundle("org.eclipse.compare.contentmergeviewer.TextMergeViewerResources", //$NON-NLS-1$ @@ -631,7 +652,10 @@ public void testShowUnifiedDiffActionIsFullyDescribed() { }; Utilities.initAction(action, bundle, "action.ShowUnifiedDiff."); //$NON-NLS-1$ - assertEquals("Show Unified Diff", action.getText(), "the action needs a label"); //$NON-NLS-1$ //$NON-NLS-2$ + // A failed lookup yields the key, so the label is checked against that rather + // than against the English text, which a translation would replace. + assertNotEquals("action.ShowUnifiedDiff.label", action.getText(), "the action needs a label"); //$NON-NLS-1$ //$NON-NLS-2$ + assertFalse(action.getText().isBlank(), "the action needs a label"); //$NON-NLS-1$ assertNotNull(action.getToolTipText(), "the action needs a tooltip"); //$NON-NLS-1$ assertNotNull(action.getImageDescriptor(), "the action needs an icon"); //$NON-NLS-1$ }