From f40e654e977c3f9a22534bedcef60f1408151afd Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 28 Aug 2026 12:01:04 +0200 Subject: [PATCH 1/2] Find/Replace overlay: test its key handling against a real editor The overlay's existing tests drive it on a bare text viewer, which has no workbench part behind it. Everything about how the overlay and its host editor compete for a keystroke depends on there being a part, so none of it is currently covered: the overlay could start letting the editor act on keys typed into its input fields without any test noticing. Adds an end-to-end test that opens a real editor and reaches the overlay through FindReplaceAction, the way a user does. It observes only where a command ends up taking effect, never how that is arranged, so that it keeps describing what the overlay owes its users regardless of how the arbitration is implemented: keys typed into an input field do not act on the document, Select All applies to the focused field, Enter finds in the search field and replaces in the replace one, window commands such as Save still reach the editor, the editor stays the active part, and it acts on its keys again once the overlay loses focus. Key strokes are delivered by notifying the widget rather than by posting native events, so the display filter the key binding dispatcher installs is exercised without depending on the operating system to deliver anything. Because that path is easy to get wrong in a way that would make the test vacuous, the test also asserts that a key the overlay does bind takes effect. A real editor is needed but the workspace and the IDE are not, so the test contributes a minimal editor of its own rather than opening a file. Extensions from this bundle were previously ignored because it was not marked as a singleton. Assisted-by: Claude Opus 5 --- .../META-INF/MANIFEST.MF | 2 +- .../build.properties | 1 + .../plugin.xml | 15 + .../FindReplaceOverlayInEditorTest.java | 300 ++++++++++++++++++ .../overlay/TestTextEditor.java | 79 +++++ .../overlay/TestTextEditorInput.java | 63 ++++ .../tests/WorkbenchTextEditorTestSuite.java | 2 + 7 files changed, 461 insertions(+), 1 deletion(-) create mode 100644 tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml create mode 100644 tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java create mode 100644 tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java create mode 100644 tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF index 4aaef1babe3..248700019c5 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF @@ -1,7 +1,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Plugin.name -Bundle-SymbolicName: org.eclipse.ui.workbench.texteditor.tests +Bundle-SymbolicName: org.eclipse.ui.workbench.texteditor.tests;singleton:=true Bundle-Version: 3.15.0.qualifier Bundle-Vendor: %Plugin.providerName Bundle-Localization: plugin diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/build.properties b/tests/org.eclipse.ui.workbench.texteditor.tests/build.properties index f9a3ba9a689..7077b165d0e 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/build.properties +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/build.properties @@ -13,6 +13,7 @@ # Mickael Istria (Red Hat Inc.) - 419531 Get rid of nested jars ############################################################################### bin.includes = plugin.properties,\ + plugin.xml,\ test.xml,\ about.html,\ .,\ diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml b/tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml new file mode 100644 index 00000000000..8ac744fdebd --- /dev/null +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml @@ -0,0 +1,15 @@ + + + + + + + + + + diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java new file mode 100644 index 00000000000..05e6df9c147 --- /dev/null +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java @@ -0,0 +1,300 @@ +/******************************************************************************* + * Copyright (c) 2026 Vector Informatik GmbH 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.ui.internal.findandreplace.overlay; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ResourceBundle; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Event; +import org.eclipse.swt.widgets.Text; + +import org.eclipse.jface.text.IDocument; +import org.eclipse.jface.text.ITextSelection; + +import org.eclipse.ui.IWorkbenchPage; +import org.eclipse.ui.PartInitException; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.handlers.IHandlerService; + +import org.eclipse.ui.texteditor.FindReplaceAction; +import org.eclipse.ui.texteditor.StatusTextEditor; + +/** + * End-to-end test for whether a command acts on the Find/Replace overlay or on + * the editor it is placed on. + *

+ * The overlay's input fields sit inside the editor's widget tree, so the editor + * remains the active part while a search term is typed, and its key bindings and + * handlers would otherwise carry out keys meant for an input field. How that is + * prevented is an implementation concern and deliberately not observed here; only + * where a command ends up taking effect is. + *

+ * Key strokes are delivered by notifying the widget rather than by posting native + * events, which still runs the display filter the key binding dispatcher installs + * but does not depend on the operating system, so the test also works headless. + * The native editing inside the SWT text widget does not happen that way, so the + * assertions say what must not reach the editor, plus one control + * proving that keys are dispatched at all. + *

+ * Unlike {@link FindReplaceOverlayTest}, which uses a bare viewer, this needs a + * real editor part: without one there is nothing to arbitrate. + */ +public class FindReplaceOverlayInEditorTest { + + private static final String CONTENT = "word one word two"; //$NON-NLS-1$ + + /** Tagged onto the widgets under {@link FindReplaceOverlay#ID_DATA_KEY}. */ + private static final String SEARCH_FIELD = "searchInput"; //$NON-NLS-1$ + + private static final String REPLACE_FIELD = "replaceInput"; //$NON-NLS-1$ + + private StatusTextEditor editor; + + private Text searchField; + + @BeforeEach + void openEditorWithOverlay() throws PartInitException { + PlatformUI.getWorkbench().getWorkbenchWindows()[0].getShell().forceActive(); + IWorkbenchPage page = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(); + editor = (StatusTextEditor) page.openEditor(new TestTextEditorInput(CONTENT), TestTextEditor.ID); + processPendingEvents(); + + // Opening through the action rather than through its internals also asserts + // that an editor of this kind gets the overlay rather than the dialog. + new FindReplaceAction(ResourceBundle.getBundle("org.eclipse.ui.texteditor.ConstructedEditorMessages"), //$NON-NLS-1$ + "Editor.FindReplace.", editor).run(); //$NON-NLS-1$ + processPendingEvents(); + + searchField = focusedInputField("opening the overlay is expected to focus its search field", //$NON-NLS-1$ + SEARCH_FIELD); + } + + @AfterEach + void closeEditor() { + searchField = null; + if (editor != null) { + editor.getSite().getPage().closeEditor(editor, false); + editor = null; + } + } + + /** + * A key typed into an input field belongs to that field: it must never be + * carried out on the editor's document instead. + */ + @Test + public void testKeysTypedIntoTheOverlayDoNotReachTheEditor() { + focusSearchField(); + int selectionLengthBefore = editorSelectionLength(); + + type(searchField, SWT.MOD1, 'a'); + + assertEquals(selectionLengthBefore, editorSelectionLength(), + "Select All's key binding must not select the editor's document"); //$NON-NLS-1$ + assertEquals(CONTENT, documentText(), "the document must not change"); //$NON-NLS-1$ + + type(searchField, SWT.MOD1, SWT.DEL); + + assertEquals(CONTENT, documentText(), + "delete-next-word's key binding must not delete from the document"); //$NON-NLS-1$ + + // Control: a key the overlay does bind must take effect, otherwise the + // assertions above would hold simply because no key was dispatched at all. + assertTrue(searchField.isVisible(), "precondition: the overlay is open"); //$NON-NLS-1$ + type(searchField, SWT.NONE, SWT.ESC); + assertFalse(searchField.isVisible(), "Escape in the search field must close the overlay"); //$NON-NLS-1$ + } + + /** + * The retargetable global actions must act on the focused input field, so that + * Edit > Select All does what the user expects while typing a search term. + */ + @Test + public void testSelectAllActsOnTheFocusedInputField() throws Exception { + focusSearchField(); + searchField.setText("abc"); //$NON-NLS-1$ + searchField.setSelection(0, 0); + int editorSelectionBefore = editorSelectionLength(); + + executeCommand("org.eclipse.ui.edit.selectAll"); //$NON-NLS-1$ + + assertEquals(3, searchField.getSelectionCount(), + "Select All must select the text of the focused input field"); //$NON-NLS-1$ + assertEquals(editorSelectionBefore, editorSelectionLength(), + "Select All must not select the editor's document"); //$NON-NLS-1$ + } + + /** + * Enter finds the next match in the search field and replaces in the replace + * field, so the two must be told apart although the key is the same. + */ + @Test + public void testEnterMeansSomethingElseInEachInputField() throws Exception { + focusSearchField(); + searchField.setText("word"); //$NON-NLS-1$ + processPendingEvents(); + int firstMatch = editorSelectionOffset(); + + type(searchField, SWT.NONE, SWT.CR); + + assertNotEquals(firstMatch, editorSelectionOffset(), + "Enter in the search field must move on to the next match"); //$NON-NLS-1$ + assertEquals(CONTENT, documentText(), "finding must not change the document"); //$NON-NLS-1$ + + Text replaceField = showReplaceField(); + replaceField.setText("X"); //$NON-NLS-1$ + processPendingEvents(); + + type(replaceField, SWT.NONE, SWT.CR); + + assertNotEquals(CONTENT, documentText(), + "Enter in the replace field must replace the current match in the document"); //$NON-NLS-1$ + } + + /** + * Commands of the surrounding window, Save among them, must stay executable + * while an input field has focus: only the editor's own are out of place there. + */ + @Test + public void testWindowCommandsStillWorkWhileTheOverlayHasFocus() throws Exception { + document().set("edited"); //$NON-NLS-1$ + processPendingEvents(); + assertTrue(editor.isDirty(), "precondition: the editor has unsaved changes"); //$NON-NLS-1$ + focusSearchField(); + + executeCommand("org.eclipse.ui.file.save"); //$NON-NLS-1$ + + assertFalse(editor.isDirty(), "Save must still reach the editor while the overlay has focus"); //$NON-NLS-1$ + } + + /** + * The overlay is not a part of its own as far as the workbench is concerned, so + * views tracking the active part or the selection must see nothing change. + */ + @Test + public void testTheEditorStaysTheActivePartWhileTheOverlayHasFocus() { + focusSearchField(); + IWorkbenchPage page = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(); + + assertEquals(editor, page.getActiveEditor(), "active editor"); //$NON-NLS-1$ + assertEquals(editor, page.getActivePart(), "active part"); //$NON-NLS-1$ + assertNotNull(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getSelectionService().getSelection(), + "the selection service must keep reporting the editor's selection"); //$NON-NLS-1$ + } + + /** + * The editor acts on its keys again once the overlay no longer has focus. Only + * asserting the other direction would be satisfied by an overlay that disabled + * the editor's keys for good. + */ + @Test + public void testTheEditorActsOnKeysAgainOnceTheOverlayLostFocus() { + focusSearchField(); + editor.setFocus(); + processPendingEvents(); + + type(editorWidget(), SWT.MOD1, 'a'); + + assertEquals(CONTENT.length(), editorSelectionLength(), + "Select All must apply to the document again once the editor has focus"); //$NON-NLS-1$ + } + + private void focusSearchField() { + // Focus the editor first, so that focusing the search field is a real + // transition rather than a no-op on an already focused control. + editor.setFocus(); + processPendingEvents(); + searchField.forceFocus(); + processPendingEvents(); + assertTrue(searchField.isFocusControl(), "the search field is expected to have focus"); //$NON-NLS-1$ + } + + /** Reveals the replace field through the overlay's own command, which focuses it. */ + private Text showReplaceField() throws Exception { + executeCommand(FindReplaceOverlayCommandSupport.CMD_TOGGLE_REPLACE); + return focusedInputField("showing the replace field is expected to focus it", //$NON-NLS-1$ + REPLACE_FIELD); + } + + private static Text focusedInputField(String message, String expectedId) { + Text field = assertInstanceOf(Text.class, Display.getCurrent().getFocusControl(), message); + assertEquals(expectedId, field.getParent().getData(FindReplaceOverlay.ID_DATA_KEY), message); + return field; + } + + private static void executeCommand(String commandId) throws Exception { + PlatformUI.getWorkbench().getService(IHandlerService.class).executeCommand(commandId, null); + processPendingEvents(); + } + + /** + * Delivers a key stroke the way the workbench sees it, through the display + * filter the key binding dispatcher installs. + */ + private static void type(org.eclipse.swt.widgets.Control target, int stateMask, int keyCode) { + Event keyEvent = new Event(); + keyEvent.widget = target; + keyEvent.type = SWT.KeyDown; + keyEvent.stateMask = stateMask; + keyEvent.keyCode = keyCode; + // Control, and only Control, turns a letter into a control character. On macOS + // SWT.MOD1 is Command, which does not. + keyEvent.character = (char) ((stateMask & SWT.CTRL) != 0 && Character.isLetter(keyCode) + ? Character.toUpperCase(keyCode) - 64 + : keyCode); + target.notifyListeners(SWT.KeyDown, keyEvent); + processPendingEvents(); + } + + private org.eclipse.swt.widgets.Control editorWidget() { + return editor.getAdapter(org.eclipse.jface.text.ITextViewer.class).getTextWidget(); + } + + private IDocument document() { + return editor.getDocumentProvider().getDocument(editor.getEditorInput()); + } + + private String documentText() { + return document().get(); + } + + private ITextSelection editorSelection() { + return (ITextSelection) editor.getSelectionProvider().getSelection(); + } + + private int editorSelectionLength() { + return editorSelection().getLength(); + } + + private int editorSelectionOffset() { + return editorSelection().getOffset(); + } + + private static void processPendingEvents() { + Display display = Display.getCurrent(); + while (display != null && !display.isDisposed() && display.readAndDispatch()) { + // keep dispatching + } + } + +} diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java new file mode 100644 index 00000000000..a7fd00c5767 --- /dev/null +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java @@ -0,0 +1,79 @@ +/******************************************************************************* + * Copyright (c) 2026 Vector Informatik GmbH 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.ui.internal.findandreplace.overlay; + +import org.eclipse.core.runtime.IProgressMonitor; + +import org.eclipse.jface.operation.IRunnableContext; + +import org.eclipse.jface.text.Document; +import org.eclipse.jface.text.IDocument; +import org.eclipse.jface.text.source.IAnnotationModel; + +import org.eclipse.ui.texteditor.AbstractDocumentProvider; +import org.eclipse.ui.texteditor.StatusTextEditor; + +/** + * A minimal text editor for tests that need a real editor part rather than a bare + * viewer, without pulling in the workspace or the IDE: its input is held in memory + * and nothing is ever saved. Contributed by this bundle's {@code plugin.xml} and + * opened by {@link #ID}, with no file name or content type association, so it is + * never offered for a real file. + */ +public class TestTextEditor extends StatusTextEditor { + + public static final String ID = "org.eclipse.ui.workbench.texteditor.tests.testTextEditor"; //$NON-NLS-1$ + + public TestTextEditor() { + setDocumentProvider(new InMemoryDocumentProvider()); + // Normally established by AbstractDecoratedTextEditor, which belongs to a higher + // layer. This scope is what makes an editor a text editor for key bindings. + setKeyBindingScopes(new String[] { "org.eclipse.ui.textEditorScope" }); //$NON-NLS-1$ + } + + private static final class InMemoryDocumentProvider extends AbstractDocumentProvider { + + @Override + protected IDocument createDocument(Object element) { + return new Document(element instanceof TestTextEditorInput input ? input.getContent() : ""); //$NON-NLS-1$ + } + + @Override + protected IAnnotationModel createAnnotationModel(Object element) { + return null; + } + + @Override + protected void doSaveDocument(IProgressMonitor monitor, Object element, IDocument document, + boolean overwrite) { + // nothing to save, the document only lives for the duration of a test + } + + @Override + protected IRunnableContext getOperationRunner(IProgressMonitor monitor) { + return null; + } + + // AbstractDocumentProvider defaults to read-only, for which the overlay hides + // its replace field, unlike the editors it is actually used with. + + @Override + public boolean isReadOnly(Object element) { + return false; + } + + @Override + public boolean isModifiable(Object element) { + return true; + } + } + +} diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java new file mode 100644 index 00000000000..0e094e9d7ff --- /dev/null +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java @@ -0,0 +1,63 @@ +/******************************************************************************* + * Copyright (c) 2026 Vector Informatik GmbH 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.ui.internal.findandreplace.overlay; + +import org.eclipse.jface.resource.ImageDescriptor; + +import org.eclipse.ui.IEditorInput; +import org.eclipse.ui.IPersistableElement; + +/** + * Input for {@link TestTextEditor}, holding its content in memory. + */ +public class TestTextEditorInput implements IEditorInput { + + private final String content; + + public TestTextEditorInput(String content) { + this.content = content; + } + + public String getContent() { + return content; + } + + @Override + public boolean exists() { + return true; + } + + @Override + public ImageDescriptor getImageDescriptor() { + return ImageDescriptor.getMissingImageDescriptor(); + } + + @Override + public String getName() { + return "Test"; //$NON-NLS-1$ + } + + @Override + public IPersistableElement getPersistable() { + return null; + } + + @Override + public String getToolTipText() { + return getName(); + } + + @Override + public T getAdapter(Class adapter) { + return null; + } + +} diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java index 492442c11be..3ae8d48c338 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java @@ -18,6 +18,7 @@ import org.eclipse.ui.internal.findandreplace.FindReplaceLogicTest; import org.eclipse.ui.internal.findandreplace.HistoryStoreTest; +import org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlayInEditorTest; import org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlayTest; import org.eclipse.ui.workbench.texteditor.tests.minimap.MinimapPageTest; @@ -48,6 +49,7 @@ TextViewerDeleteLineTargetTest.class, FindReplaceLogicTest.class, FindReplaceOverlayTest.class, + FindReplaceOverlayInEditorTest.class, FindReplaceDialogTest.class, HistoryStoreTest.class, }) From 4cce1ed411801a1c1543d48766265f6397db7e70 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 28 Aug 2026 12:02:57 +0200 Subject: [PATCH 2/2] Find/Replace overlay: make the overlay the active part while focused The overlay's control is parented into the editor's widget tree, so focusing an input field does not change the active part. The editor's key bindings and command handlers therefore stayed in effect and consumed keys meant for the input fields, which was worked around by reflectively disabling the editor's action activation and by nulling out its global action handlers. That workaround reached into private API, applied only to AbstractTextEditor, and covered only the six retargetable actions rather than the full set of conflicting commands. Instead of suppressing the editor's commands one by one, the overlay now takes the editor out of the resolution path while an input field has focus. Both conditions that decide whether one of the editor's handlers wins have to be addressed, because the editor's commands are spread over both: its key binding scopes and part-level handlers are reachable through the editor's part context, while its retargetable actions live in the window context and are guarded by an expression over the active part id. Activating a context that is a sibling of the editor's part context removes the former, and declaring that context's own id as the active part id makes the latter evaluate to false. The context is placed below the window context rather than below the application context, so that window-scoped commands and services remain available. Only the active part id is overridden, not the active part itself, so the overlay and anything invoked from it still operate on the editor, and so do contributions keyed on the active part, the active editor or the selection. With no editor handler left in the resolution path, keys the platform does not otherwise handle reach the native text widget, and the workbench-wide default handlers for cut, copy, paste and select all act on the focused input field, so those also work from the Edit menu again. The overlay's key binding scopes are activated in that same context rather than at the workbench context service. Scopes are collected along the chain between the active leaf and the root, so a scope activated there is active exactly while the context is the active leaf: the overlay's shared scope is activated once and never deactivated, and only the per-field scope is switched as focus moves between the input fields. Both kinds of context are consequently owned by one class, leaving the command support with handler activation and shortcut hints. Since the overlay reports itself as the active part, that is also what scopes its own command handlers, which are activated once at the workbench. The active part id changes at exactly the moments focus enters and leaves an input field, and it says which overlay is focused, so the overlays of different editors no longer need to be told apart by inspecting the focus control's widget hierarchy and no focus tracking has to be registered for the input fields at all. The overlay's shared scope no longer declares the text editor scope as its parent. Parent scopes are resolved when building the set a binding lookup runs against, so that parent would have reintroduced the editor's bindings regardless of the context topology, and it also tied the overlay to text editors. None of this changes what the overlay owes its users, so the end-to-end test added by the preceding commit passes unchanged before and after. Assisted-by: Claude Opus 5 --- .../META-INF/MANIFEST.MF | 5 +- .../plugin.xml | 2 +- .../overlay/FindReplaceOverlay.java | 4 +- .../FindReplaceOverlayCommandSupport.java | 197 ++---------- .../FindReplaceOverlayContextSupport.java | 223 ++++++++++++++ .../0001-find-replace-overlay-key-handling.md | 281 ++++++++++++++++++ 6 files changed, 534 insertions(+), 178 deletions(-) create mode 100644 bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayContextSupport.java create mode 100644 docs/adr/0001-find-replace-overlay-key-handling.md diff --git a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF index 924a82d0fd8..252e3d8488d 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF @@ -33,7 +33,10 @@ Require-Bundle: org.eclipse.jface.text;bundle-version="[3.19.0,4.0.0)", org.eclipse.swt;bundle-version="[3.133.0,4.0.0)", org.eclipse.ui;bundle-version="[3.208.0,4.0.0)", - org.eclipse.jface.notifications + org.eclipse.jface.notifications, + org.eclipse.e4.core.contexts;bundle-version="1.0.0", + org.eclipse.e4.ui.model.workbench;bundle-version="1.3.0", + org.eclipse.e4.ui.services;bundle-version="1.0.0" Bundle-RequiredExecutionEnvironment: JavaSE-21 Automatic-Module-Name: org.eclipse.ui.workbench.texteditor Require-Capability: eclipse.swt;filter:="(image.format=svg)" diff --git a/bundles/org.eclipse.ui.workbench.texteditor/plugin.xml b/bundles/org.eclipse.ui.workbench.texteditor/plugin.xml index 47d2f1a1a34..a7fcb1aca25 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/plugin.xml +++ b/bundles/org.eclipse.ui.workbench.texteditor/plugin.xml @@ -1561,7 +1561,7 @@ + parentId="org.eclipse.ui.contexts.window"> commandSupport.dispose()); customFocusOrder.install(); updateReplaceVisibility(false); containerControl.setVisible(false); @@ -446,9 +446,7 @@ private void createContentsContainer() { GridDataFactory.fillDefaults().grab(true, true).align(GridData.FILL, GridData.FILL).applyTo(contentGroup); createSearchContainer(); - commandSupport.trackFocusControl(searchBar.getTextBar()); createReplaceContainer(); - commandSupport.trackFocusControl(replaceBar.getTextBar()); } private void createSearchTools() { diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java index f2dce606d26..5cd16887072 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java @@ -10,51 +10,32 @@ *******************************************************************************/ package org.eclipse.ui.internal.findandreplace.overlay; -import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Control; -import org.eclipse.swt.widgets.Text; - -import org.eclipse.core.expressions.EvaluationResult; import org.eclipse.core.expressions.Expression; -import org.eclipse.core.expressions.ExpressionInfo; -import org.eclipse.core.expressions.IEvaluationContext; - -import org.eclipse.core.runtime.ILog; - -import org.eclipse.jface.action.IAction; -import org.eclipse.ui.IActionBars; -import org.eclipse.ui.ISources; import org.eclipse.ui.IWorkbenchPart; import org.eclipse.ui.PlatformUI; -import org.eclipse.ui.contexts.IContextActivation; -import org.eclipse.ui.contexts.IContextService; import org.eclipse.ui.handlers.IHandlerActivation; import org.eclipse.ui.handlers.IHandlerService; -import org.eclipse.ui.part.MultiPageEditorSite; -import org.eclipse.ui.swt.IFocusService; - -import org.eclipse.ui.texteditor.AbstractTextEditor; -import org.eclipse.ui.texteditor.ITextEditorActionConstants; /** - * Owns the Find/Replace overlay's command infrastructure, including context - * activation, handler activation, and key-binding hint updates. + * Owns the Find/Replace overlay's command infrastructure: handler activation and + * key-binding hint updates. + *

+ * The overlay's own commands are activated as handlers once, rather than + * imperatively activated and deactivated on every focus change. They are scoped + * by {@link FindReplaceOverlayContextSupport#overlayFocusedExpression()}, which + * both limits them to the time an input field has focus and tells them apart + * from the handlers of the overlays of other editors, since all of those are + * activated at the workbench for the same commands. *

- * The overlay's own commands are activated as handlers once, scoped by - * {@link #overlayFocusedExpression}, rather than imperatively - * activated/deactivated on every focus change. That expression relies on - * {@link IFocusService} tracking the search/replace bar controls so that - * {@code ACTIVE_FOCUS_CONTROL} reflects them. Context activation (which - * drives key binding resolution and has no expression-based equivalent) - * remains imperative and is updated directly from the overlay's focus - * listeners. + * Everything context related, both the overlay's key binding scopes and keeping + * the editor's own commands from consuming keys meant for the input fields, is + * owned by {@link FindReplaceOverlayContextSupport}. This class only forwards + * the overlay's focus changes to it, because the shortcut hints have to be + * refreshed whenever the active scopes change. */ class FindReplaceOverlayCommandSupport { @@ -81,70 +62,20 @@ class FindReplaceOverlayCommandSupport { static final String CMD_REPLACE_ALL = "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.replaceAll"; //$NON-NLS-1$ - private static final String OVERLAY_CONTEXT_ID = - "org.eclipse.ui.workbench.texteditor.findReplaceOverlay"; //$NON-NLS-1$ - private static final String OVERLAY_SEARCH_CONTEXT_ID = - "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.searchFocused"; //$NON-NLS-1$ - private static final String OVERLAY_REPLACE_CONTEXT_ID = - "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.replaceFocused"; //$NON-NLS-1$ - - private Composite containerControl; - private final IWorkbenchPart targetPart; - private DeactivateGlobalActionHandlers globalActionHandlerDeaction; - - private final List contextActivations = new ArrayList<>(); private final Expression overlayFocusedExpression; + private final FindReplaceOverlayContextSupport contextSupport; private final List registeredActions = new ArrayList<>(); private final List actionActivations = new ArrayList<>(); FindReplaceOverlayCommandSupport(IWorkbenchPart targetPart) { - this.targetPart = targetPart; - this.overlayFocusedExpression = createOverlayFocusedExpression(); - } - - private Expression createOverlayFocusedExpression() { - return new Expression() { - @Override - public EvaluationResult evaluate(IEvaluationContext context) { - Object focusControl = context.getVariable(ISources.ACTIVE_FOCUS_CONTROL_NAME); - if (focusControl instanceof Control control) { - Control current = control; - while (current != null) { - if (current == containerControl) { - return EvaluationResult.TRUE; - } - current = current.getParent(); - } - } - return EvaluationResult.FALSE; - } - - @Override - public void collectExpressionInfo(ExpressionInfo info) { - info.addVariableNameAccess(ISources.ACTIVE_FOCUS_CONTROL_NAME); - } - }; + this.contextSupport = new FindReplaceOverlayContextSupport(targetPart); + this.overlayFocusedExpression = contextSupport.overlayFocusedExpression(); } - void trackFocusControl(Text text) { - IFocusService focusService = PlatformUI.getWorkbench().getService(IFocusService.class); - if (focusService != null) { - focusService.addFocusTracker(text, "" + text.hashCode()); //$NON-NLS-1$ - } - } - - void setContainerControl(Composite containerControl) { - this.containerControl = containerControl; - containerControl.addDisposeListener(__ -> { - deregisterActionActivations(); - // Safety net: normally already done by the focus-lost handling that runs - // while the overlay is closed via close(), but disposal is not guaranteed - // to be preceded by a focus-lost event, so repeat it here defensively. Both - // calls are idempotent if that cleanup already ran. - deactivateContexts(); - setTextEditorActionsActivated(true); - }); + void dispose() { + deregisterActionActivations(); + contextSupport.dispose(); } void registerAction(FindReplaceOverlayAction action) { @@ -178,104 +109,24 @@ private static IHandlerService getWorkbenchHandlerService() { } void searchBarActivated() { - searchOrReplaceBarActivated(OVERLAY_SEARCH_CONTEXT_ID); + contextSupport.searchBarFocused(); + refreshShortcutHints(); } void replaceBarActivated() { - searchOrReplaceBarActivated(OVERLAY_REPLACE_CONTEXT_ID); - } - - private void searchOrReplaceBarActivated(String barContextId) { - setTextEditorActionsActivated(false); - // Defensively clear any contexts still active from a previous activation, - // making this method idempotent instead of relying on a focus-lost event - // always having deactivated them first. - deactivateContexts(); - activateContext(OVERLAY_CONTEXT_ID); - activateContext(barContextId); + contextSupport.replaceBarFocused(); refreshShortcutHints(); } - private void activateContext(String context) { - IContextService contextService = getWorkbenchContextService(); - if (contextService != null) { - contextActivations.add(contextService.activateContext(context)); - } - } - - private static IContextService getWorkbenchContextService() { - return PlatformUI.getWorkbench().getService(IContextService.class); - } - void searchOrReplaceBarDeactivated() { - deactivateContexts(); - setTextEditorActionsActivated(true); + contextSupport.fieldsLostFocus(); refreshShortcutHints(); } - private void deactivateContexts() { - IContextService contextService = getWorkbenchContextService(); - if (contextService != null) { - for (IContextActivation activation : contextActivations.reversed()) { - contextService.deactivateContext(activation); - } - } - contextActivations.clear(); - } - private void refreshShortcutHints() { for (FindReplaceOverlayAction action : registeredActions) { action.updateHint(); } } - /* - * Adapted from - * org.eclipse.jdt.internal.ui.javaeditor.JavaEditor#setActionsActivated(boolean) - */ - private void setTextEditorActionsActivated(boolean state) { - if (!(targetPart instanceof AbstractTextEditor) || targetPart.getSite().getWorkbenchWindow().isClosing()) { - return; - } - if (targetPart.getSite() instanceof MultiPageEditorSite multiEditorSite) { - if (!state && globalActionHandlerDeaction == null) { - globalActionHandlerDeaction = new DeactivateGlobalActionHandlers(multiEditorSite.getActionBars()); - } else if (state && globalActionHandlerDeaction != null) { - globalActionHandlerDeaction.reactivate(); - globalActionHandlerDeaction = null; - } - } - try { - Method method = AbstractTextEditor.class.getDeclaredMethod("setActionActivation", boolean.class); //$NON-NLS-1$ - method.setAccessible(true); - method.invoke(targetPart, Boolean.valueOf(state)); - } catch (IllegalArgumentException | ReflectiveOperationException ex) { - ILog.of(FindReplaceOverlayCommandSupport.class).error("cannot (de-)activate actions for text editor", ex); //$NON-NLS-1$ - } - } - - private static final class DeactivateGlobalActionHandlers { - private static final List ACTIONS = List.of(ITextEditorActionConstants.CUT, - ITextEditorActionConstants.COPY, ITextEditorActionConstants.PASTE, - ITextEditorActionConstants.DELETE, ITextEditorActionConstants.SELECT_ALL, - ITextEditorActionConstants.FIND); - - private final Map deactivatedActions = new HashMap<>(); - private final IActionBars actionBars; - - DeactivateGlobalActionHandlers(IActionBars actionBars) { - this.actionBars = actionBars; - for (String actionID : ACTIONS) { - deactivatedActions.putIfAbsent(actionID, actionBars.getGlobalActionHandler(actionID)); - actionBars.setGlobalActionHandler(actionID, null); - } - } - - void reactivate() { - for (String actionID : deactivatedActions.keySet()) { - actionBars.setGlobalActionHandler(actionID, deactivatedActions.get(actionID)); - } - } - } - } diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayContextSupport.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayContextSupport.java new file mode 100644 index 00000000000..9611b7d7e9b --- /dev/null +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayContextSupport.java @@ -0,0 +1,223 @@ +/******************************************************************************* + * Copyright (c) 2026 Vector Informatik GmbH 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.ui.internal.findandreplace.overlay; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.eclipse.core.expressions.EvaluationResult; +import org.eclipse.core.expressions.Expression; +import org.eclipse.core.expressions.ExpressionInfo; +import org.eclipse.core.expressions.IEvaluationContext; + +import org.eclipse.e4.core.contexts.IEclipseContext; +import org.eclipse.e4.ui.model.application.ui.basic.MBasicFactory; +import org.eclipse.e4.ui.model.application.ui.basic.MPart; +import org.eclipse.e4.ui.services.EContextService; + +import org.eclipse.ui.ISources; +import org.eclipse.ui.IWorkbenchPart; +import org.eclipse.ui.IWorkbenchPartSite; +import org.eclipse.ui.IWorkbenchWindow; +import org.eclipse.ui.PlatformUI; + +/** + * Owns the Find/Replace overlay's contexts, of which there are two kinds: an + * {@link IEclipseContext} that makes the overlay rather than its host editor the + * active part while an input field has focus, and the overlay's key binding + * scopes. The second kind lives inside the first, because scopes are collected + * along the chain between the active leaf and the root: a scope activated here is + * active exactly while this context is the active leaf, so the shared scope is + * activated once and only the per-field scope is switched. + *

+ * Two conditions decide whether one of the editor's handlers wins, and the + * editor's commands are spread over both. Its key binding scopes and part-level + * handlers are only reachable through the editor's part context, which + * activating a sibling of that context takes off the chain. Its retargetable + * actions live in the window context instead, guarded by an expression + * over {@link ISources#ACTIVE_PART_ID_NAME}, which publishing this context's own + * id as the active part id makes false. Together they leave no editor handler in + * the resolution path, so no command has to be suppressed individually. The + * context sits below the window context rather than the application context, + * which would detach window-scoped commands and services as well. + *

+ * The {@link MPart} looks superfluous, since nothing reads it, but removing it + * changes behaviour: it is what lets {@code ActivePartLookupFunction} resolve an + * active part here. Without it that lookup yields {@code null}, and + * {@code PartServiceImpl} answers a null active part by firing part deactivation + * and clearing the active selection. With it, the same code path finds a part + * outside the application model and returns early. That early return is also why + * the part is neither added to the model nor rendered nor activated through + * {@code EPartService}. + *

+ * The alternatives considered, and what was measured about them, are recorded in + * {@code docs/adr/0001-find-replace-overlay-key-handling.md}. + */ +class FindReplaceOverlayContextSupport { + + static final String OVERLAY_PART_ID_PREFIX = "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.part."; //$NON-NLS-1$ + + /** + * Tells the overlays of different editors apart: their command handlers are all + * activated at the workbench, so the id must not be shared. + */ + private static final AtomicInteger PART_ID_SEQUENCE = new AtomicInteger(); + + private static final String OVERLAY_CONTEXT_ID = "org.eclipse.ui.workbench.texteditor.findReplaceOverlay"; //$NON-NLS-1$ + + private static final String OVERLAY_SEARCH_CONTEXT_ID = "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.searchFocused"; //$NON-NLS-1$ + + private static final String OVERLAY_REPLACE_CONTEXT_ID = "org.eclipse.ui.workbench.texteditor.findReplaceOverlay.replaceFocused"; //$NON-NLS-1$ + + private final IWorkbenchPart targetPart; + + private final String overlayPartId = OVERLAY_PART_ID_PREFIX + PART_ID_SEQUENCE.incrementAndGet(); + + private IEclipseContext overlayContext; + + /** + * The leaf that was active before the overlay took over, restored when it gives + * focus back. Remembered rather than derived from the target part, which also + * works without a part and does not need the part's site to still be there. Held + * only while the overlay owns the active leaf, so that handing it back does not + * keep the context of an editor closed in the meantime reachable. + */ + private IEclipseContext contextToRestore; + + private boolean overlayContextActive; + + private final Expression overlayFocusedExpression = createOverlayFocusedExpression(); + + /** + * The per-field scope currently activated in {@link #overlayContext}. Only ever + * switched: while that context is off the active chain its scopes are inert. + */ + private String activeFieldContextId; + + FindReplaceOverlayContextSupport(IWorkbenchPart targetPart) { + this.targetPart = targetPart; + this.overlayContext = createOverlayContext(); + } + + private IEclipseContext createOverlayContext() { + IEclipseContext windowContext = getWindowContext(); + if (windowContext == null) { + return null; + } + MPart overlayPart = MBasicFactory.INSTANCE.createPart(); + overlayPart.setElementId(overlayPartId); + + IEclipseContext context = windowContext.createChild(overlayPartId); + context.set(MPart.class, overlayPart); + // Only the id is overridden, not ACTIVE_PART_NAME: the overlay operates on the + // editor, and so does anything invoked while the overlay has focus, so the + // active part itself must keep pointing at the editor. + context.set(ISources.ACTIVE_PART_ID_NAME, overlayPartId); + overlayPart.setContext(context); + + context.get(EContextService.class).activateContext(OVERLAY_CONTEXT_ID); + return context; + } + + /** + * Holds exactly while one of this overlay's input fields has focus, for scoping + * activations to this overlay alone. It tests the active part id this context + * publishes, which is unique per overlay. Declaring that variable in + * {@code collectExpressionInfo} is what gives such activations their source + * priority, so the expression belongs next to the code setting the variable. + */ + Expression overlayFocusedExpression() { + return overlayFocusedExpression; + } + + private Expression createOverlayFocusedExpression() { + return new Expression() { + @Override + public EvaluationResult evaluate(IEvaluationContext context) { + return EvaluationResult + .valueOf(overlayPartId.equals(context.getVariable(ISources.ACTIVE_PART_ID_NAME))); + } + + @Override + public void collectExpressionInfo(ExpressionInfo info) { + info.addVariableNameAccess(ISources.ACTIVE_PART_ID_NAME); + } + }; + } + + void searchBarFocused() { + fieldFocused(OVERLAY_SEARCH_CONTEXT_ID); + } + + void replaceBarFocused() { + fieldFocused(OVERLAY_REPLACE_CONTEXT_ID); + } + + private void fieldFocused(String fieldContextId) { + if (overlayContext == null) { + return; + } + if (!fieldContextId.equals(activeFieldContextId)) { + EContextService contextService = overlayContext.get(EContextService.class); + if (activeFieldContextId != null) { + contextService.deactivateContext(activeFieldContextId); + } + contextService.activateContext(fieldContextId); + activeFieldContextId = fieldContextId; + } + if (!overlayContextActive) { + contextToRestore = overlayContext.getParent().getActiveLeaf(); + overlayContextActive = true; + } + overlayContext.activate(); + } + + /** + * Hands the active leaf back to whatever held it before the overlay took over. + * Activating the overlay's context replaced the window context's active child, + * so the whole chain down to that context has to be re-established, not just its + * last link. + */ + void fieldsLostFocus() { + if (!overlayContextActive) { + return; + } + overlayContextActive = false; + if (contextToRestore != null) { + contextToRestore.activateBranch(); + contextToRestore = null; + } + } + + void dispose() { + if (overlayContext != null) { + fieldsLostFocus(); + overlayContext.dispose(); + overlayContext = null; + } + } + + /** + * The window the overlay belongs to. Only when there is no part at all, which + * the find/replace UI tests exercise, does the overlay fall back to the active + * window rather than guessing one for a part whose site is unavailable. + */ + private IEclipseContext getWindowContext() { + IWorkbenchWindow window; + if (targetPart == null) { + window = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); + } else { + IWorkbenchPartSite site = targetPart.getSite(); + window = site == null ? null : site.getWorkbenchWindow(); + } + return window == null ? null : window.getService(IEclipseContext.class); + } + +} diff --git a/docs/adr/0001-find-replace-overlay-key-handling.md b/docs/adr/0001-find-replace-overlay-key-handling.md new file mode 100644 index 00000000000..d46cddc9f9b --- /dev/null +++ b/docs/adr/0001-find-replace-overlay-key-handling.md @@ -0,0 +1,281 @@ +# 1. Find/Replace overlay: key event and editor command handling + +How the Find/Replace overlay makes sure that the right commands are in effect while one of its +input fields has focus: its own, and those of the surrounding workbench, but not those of the +editor it is drawn on, whose key bindings and handlers would otherwise carry out edits on the +document rather than in the field the user is typing into. The overlay is not a workbench part, +so the framework does not separate the two on its own. This decision covers how that separation +is achieved, and how it is achieved without tying the overlay to a particular kind of editor. + +## Status + +Accepted. + +## Context + +The Find/Replace overlay shows text input fields on top of a text editor. Its `containerControl` +is a plain SWT `Composite` parented into the editor's widget tree. + +Being embedded rather than living in its own `Shell` is a hard requirement: a shell has to +manually follow the target widget's move, resize and hide/show operations, always lags behind by +some milliseconds, and cannot be positioned at all under Wayland (see commit `78c9a1c60a`, which +introduced the embedded composite for these reasons). **A separate shell is therefore out of +scope**, which constrains the whole design space below. + +The consequence of being embedded is the root of the problem: focus in the overlay's fields does +not change the active workbench part. The editor stays active, so its key-binding scopes and its +command handlers stay in effect and compete with the overlay for every keystroke. + +### Goals + +1. Standard text editing keys (Ctrl+C/V/X/Z/A, Ctrl+Backspace, word navigation, Home/End, + arrows, Delete) must operate on the overlay's input fields, not on the editor's document. +2. The retargetable global actions (Edit > Cut/Copy/Paste/Select All) must be bound to the + overlay's fields while they have focus, so the Edit menu and toolbar act on them. +3. Commands in the "In Windows" scope (Save, Close, Next Editor, Preferences, ...) must stay + executable while the overlay has focus. +4. The overlay must not depend on `AbstractTextEditor` or `StatusTextEditor`, so it can later + serve non-text editors. +5. No reflective access to private platform API. + +### How Eclipse decides which handler runs + +Two independent gates decide whether a handler activation wins. + +**Gate 1, reachability.** `HandlerServiceImpl#lookUpHandler` resolves +`context.getActiveLeaf().get("handler::" + commandId)`, and the `HandlerSelectionFunction` +installed there walks from that leaf up to the root. An activation in a context that is not an +ancestor of the active leaf is never considered. Key-binding scopes work the same way: +`ActiveContextsFunction` unions the `localContexts` of every context from the active leaf +upwards, and that union is what `ContextManager` and the E4 `BindingService` use. + +**Gate 2, expression.** Among reachable activations, only those whose `activeWhen` expression +evaluates to true participate. The winner is chosen by `HandlerActivation#compareTo` on source +priorities computed by `SourcePriorityNameMapping#computeSourcePriority` from the variables the +expression accesses. There is no part/window layering in this comparison; `getDepth()` is +always `0`. Note that `activeFocusControl` maps to `ISources.ACTIVE_MENU` (`1 << 31`), which +`compareTo` normalises into `1 << 30`, higher than any other source. That, and not any notion +of part-level versus window-level, is why an `ACTIVE_FOCUS_CONTROL`-scoped activation outranks +the editor's. + +Legacy `ISources` variables are resolved by `ExpressionContext#getVariable` through +`IEclipseContext#getActive(name)`, that is, **from the active leaf upwards**, so a context +nearer the leaf can shadow them. + +A key reaches the widget when the winning handler reports `isHandled() == false`: +`KeyBindingDispatcher#executeCommand` computes `commandHandled` from the handler, `press()` +returns false, `processKeyEvent` leaves `event.doit` untouched, and SWT delivers the key +natively. A command with no handler at all behaves the same way. + +### Where the editor registers what + +The editor's commands do not all live in one place, which is what makes the problem non-obvious: + +| Registered by | Into which context | Guarded by | +|---|---|---| +| Editor's `KeyBindingService` (scopes such as `org.eclipse.ui.textEditorScope`) | the **part** context | n/a | +| Editor's own `IHandlerService.activateHandler` calls | the **part** context | `ActivePartExpression` | +| Retargetable actions (cut, copy, paste, delete, select all, undo, redo) via `IActionBars.setGlobalActionHandler` | the **window** context, because `EditorReference` constructs `EditorActionBars` with `page.getWorkbenchWindow()` as service locator | `LegacyEditorActionBarExpression`, comparing `activePartId` against the editor id | + +Any approach that only detaches the part context addresses the first two rows and leaves the +third untouched. The third row holds exactly the commands users notice most. + +### Context inheritance of the overlay's own scope + +`ContextSet` resolves parent contexts when building the set a binding lookup runs against. If +the overlay's own key-binding context declares `parentId="org.eclipse.ui.textEditorScope"`, the +editor scope is pulled back in **even when the editor's part context has been taken off the +active chain**, and Ctrl+Delete still resolves to `deleteNextWord`. Giving that context a parent +outside the editor scope is therefore a precondition for the chosen alternative, not cosmetic +tidying towards goal 4. + +## Decision + +While one of the overlay's input fields has focus, the overlay activates an `IEclipseContext` of +its own below the window context and publishes its own id as the active part id. That takes the +editor's key binding scopes and part-level handlers off the active context chain, and makes the +expression guarding its retargetable actions evaluate to false, so no editor handler is left in +the command resolution path and no individual command has to be suppressed. The overlay's own key +binding scopes are activated in that same context, which also gives them their lifetime, and its +shared scope is given a parent outside `org.eclipse.ui.textEditorScope`. + +### Alternatives + +**Suppress the editor's commands individually.** The editor stays the active part, and for every +command whose key an input field needs, its handler is neutralised or replaced while the field +has focus, so that the key falls through to the widget. This leaves the workbench's view of the +world untouched, since the editor remains the active part throughout. It needs a set of affected +command ids, which is either maintained by hand, and then misses commands contributed by other +plugins, or derived from the current key bindings, and then unbounded. Suppression works per +command id, so a command bound to both a native and a non-native key loses both. It also only +takes the keys away from the editor: nothing binds them to the input field, so cut, copy and +paste stay unbound and the menu entries appear enabled while doing nothing, leaving goal 2 unmet. + +**Make the overlay a workbench element of its own.** Give the overlay its own shell, or model it +as a real part and activate it, and the framework's own activation takes the editor's contexts +and handlers off the active chain. This needs no per-command work at all, and is how content +assist, Quick Access and dialogs already avoid the problem. A shell is excluded by the embedding +requirement stated in the context. A real part changes the part service's notion of the active +part, so part listeners, the selection service, Outline, link-with-editor and the editor's tab +all react, and it needs a home in the application model that a floating overlay does not +naturally have. + +**Give the overlay a context of its own (adopted).** Create an `IEclipseContext` for the overlay +and activate it while an input field has focus, without the overlay becoming a part as far as the +part service is concerned. This addresses both gates at once and needs no command set: the +editor's part-level registrations become unreachable, and its window-level ones evaluate to +false. The platform's own default handlers for cut, copy, paste and select all then act on the +focused field, which meets goal 2 without registering anything, while window-scoped commands stay +reachable. Its cost is that it relies on a divergence between the active-leaf chain and +`EPartService`'s notion of the active part which E4 does not promise to preserve, that the +`MPart` it carries exists only in a context and not in the application model, and that shadowing +`activePartId` also switches off contributions keyed on that variable against the host editor. + +### Observed behaviour + +With the search field focused in a text editor. The columns are variants described in the +appendix. + +| | Non-handling overrides | Own context, window | Own context, application | **Adopted** | +|---|---|---|---|---| +| `textEditorScope` active | yes | no | no | **no** | +| Ctrl+Delete resolves | yes | no | no | **no** | +| `edit.undo` / `edit.delete` handler | non-handling override | editor | none | **none** | +| `edit.copy` / `edit.selectAll` handler | non-handling override | editor | platform default | **platform default** | +| `deleteNextWord` handler | non-handling override | none | none | **none** | +| Save / Close / Next Editor / Preferences | ok | ok | ok | **ok** | +| `IWorkbenchWindow` from leaf | ok | ok | **null** | **ok** | +| `ISources.activePart` / `activeEditor` | editor | editor | **undefined** / editor | **editor** | +| Overlay's own bindings and handlers | ok | ok | ok | **ok** | + +## Consequences + +- The overlay does not reference `AbstractTextEditor`, and nothing in the key handling depends on + the editor type. Goal 4 is met for the command infrastructure, though `FindReplaceOverlay` + still has two `instanceof StatusTextEditor` checks (target control, colours) and + `FindReplaceAction#shouldUseOverlay()` still gates the overlay on `StatusTextEditor`, so the + overlay remains text-editor-only until those are addressed separately. +- Cut, copy, paste and select all work on the overlay's fields through the platform's default + handlers, including from the Edit menu. +- Undo, delete and the word-wise editing and navigation commands resolve to no handler while the + overlay has focus, so the native `Text` widget handles those keys. +- `org.eclipse.ui.workbench.texteditor` gains a dependency on the E4 context and model bundles. +- Per-focus imperative state is limited to activating and deactivating that one context plus + switching which of the two per-field scopes is activated inside it. The overlay's shared scope + is activated once, because scopes activated in the overlay's context are active exactly while + that context is the active leaf. + +## Appendix: variants and dead ends + +### Suppressing the editor's commands + +**Reflection into `AbstractTextEditor#setActionActivation`, plus nulling the action bars.** On +focus gained, call the private `setActionActivation(false)` reflectively, and for multi-page +editors additionally null out the `IActionBars` global action handlers for +cut/copy/paste/delete/select-all/find. Reverse both on focus lost. Addresses both registration +sites, but uses reflective private API against goal 5, is hard-coded to `AbstractTextEditor` and +therefore a no-op for any other editor against goal 4, and covers only the six global action ids, +not word navigation or undo. + +**Non-handling handler overrides.** Collect the commands bound to the ~40 key sequences that SWT +`Text` handles natively and, for each, activate a handler at the target part's `IHandlerService` +that returns `isHandled() == false`, scoped by an expression on `ACTIVE_FOCUS_CONTROL`, whose +rogue-bit priority outranks the editor's activations. Needs no reflection and is not tied to an +editor class, but each `activateHandler` also sets `handler::` in the part context through +`EHandlerService`, and `deactivateHandlers` only clears the `legacy::handler::` list, so that +entry outlives the overlay and shadows any pure-E4 handler for that command for the rest of the +editor's life. Deriving the command set from `BindingManager#getBindings()` returns every +declared binding across all schemes, platforms and locales, may return `null`, and is a snapshot +that ignores later changes in Preferences > Keys; a fixed list avoids that but misses third-party +commands. + +**`IEditorActionBarContributor#setActiveEditor(null)` on focus.** Makes the contributor call +`IActionBars.setGlobalActionHandler(id, null)` for each action it manages, removing the +window-level activations, restored on focus lost. Public API, and correct for single-page +editors, but `MultiPageEditorSite#getActionBarContributor()` returns `null` by specification and +the fallback via the outer editor does not reliably cover handlers the inner editor registered. +Addresses only the global action bar subset. + +**Explicit pass-through handlers.** Register real handlers calling the corresponding `Text` +method. Expresses intent directly and avoids the enablement problem, but SWT `Text` exposes only +`cut()`, `copy()`, `paste()` and `selectAll()`, so most of the ~30 affected operations cannot be +implemented this way, let alone correctly across platforms and locale-sensitive word boundaries. + +### Making the overlay a workbench element of its own + +**Own `Shell`.** `ShellActivationListener` gives an unmodelled shell a child context of the +application context, activates it, and activates `org.eclipse.ui.contexts.dialog`. Complete +isolation for free, but reintroduces exactly what the embedded composite exists to avoid. + +**Real `MPart` activated through `EPartService`.** The threshold is +`PartServiceImpl#isInContainer`, true once the part is found by a `PRESENTATION`-scoped model +search or tagged as a hosted element in the window's shared elements. Crossing it has two +observable effects: `PartServiceImpl` activates the part, and since it carries no compatibility +wrapper `IWorkbenchPage#getActivePart()` then reports `null` while the overlay has focus and part +deactivation is broadcast for the editor; and placing the part in the editor area additionally +renders it, which takes focus away from the input field altogether. + +### Giving the overlay a context of its own + +**Parented on the window context.** Removes `textEditorScope` and the editor's part-level +handlers with no per-command work, and does not disturb the part service. Insufficient on its +own: the retargetable actions are registered at the window handler service, the window context +remains an ancestor, and `activePartId` still resolves to the editor, so undo, copy, delete and +select all keep resolving to the editor's handlers. + +**Parented on the application context.** The topology `ShellActivationListener` gives a dialog +shell, which additionally takes the window context off the chain. Full isolation, and the +platform's default handlers surface as described in the decision, but window-scoped services stop +being injectable (`IWorkbenchWindow` resolves to `null`) and the legacy `activePart` variable +becomes undefined, so `HandlerUtil#getActivePart` and any `activeWhen` keyed on the active part +go false, putting goal 3 at risk. + +**Parented on the window context, shadowing `activePartId` (adopted).** Setting +`ISources.ACTIVE_PART_ID_NAME` to the overlay's own id in that context makes +`LegacyEditorActionBarExpression` evaluate to false, since legacy variables resolve from the +active leaf upwards. The editor's window-level activations stay reachable but stop participating, +so gate 2 does the work that detaching the window context does in the previous variant, without +its cost. Only the id is shadowed, not `ACTIVE_PART_NAME`, so anything needing a part still sees +the editor. The context activation is also stable under ordinary interaction: clicking into the +fields, moving focus to an overlay toolbar button and back, and switching between editor and +overlay by mouse all leave the overlay's context as the active leaf, despite the `SWT.Activate` +listener `ContributedPartRenderer` installs on the editor's composite. + +The `MPart` carried by the context is what lets `ActivePartLookupFunction` resolve an active part +for it. Without it that lookup yields `null`, and `PartServiceImpl` answers a null active part by +firing part deactivation and clearing the active selection. With it, the same code path finds a +part outside the application model and returns early, leaving the part service untouched. + +### Context manipulations that do not remove the editor from resolution + +- **`EContextService#deactivateContext("org.eclipse.ui.textEditorScope")` on the part's + context** removes the id from that context's `localContexts`, but the editor's + `KeyBindingService` owns and re-establishes those activations. +- **`IContextService#deactivateContext` (3.x)** needs the `IContextActivation` token, which + belongs to the editor's `KeyBindingService`. +- **`ContextManager#setActiveContextIds`** is not exposed through `IContextService`; reaching it + means casting to the internal implementation, which goal 5 excludes. +- **`IContextService#activateContext(id, falseExpression)`** cannot help, because context + activations are additive. +- **A child `IEclipseContext` of the *part* context** leaves the part context an ancestor, so + `ActiveContextsFunction` still finds `textEditorScope`. Only a context that is not a descendant + of the part context helps. + +### The focus service + +`IFocusService#addFocusTracker(control, id)` makes `ACTIVE_FOCUS_CONTROL` and +`ACTIVE_FOCUS_CONTROL_ID` reflect the overlay's fields when they have focus. It is the documented +way to attach `WidgetMethodHandler`-based cut/copy/paste to a widget outside the part lifecycle, +and it is the source whose priority lets the suppression variants outrank the editor. It changes +nothing on its own, and `FocusControlSourceProvider` clears the variables on focus lost of a +tracked control, so anything keyed on them is inactive while an overlay toolbar button has focus. +For the adopted alternative it is redundant, because `activePartId` already changes at exactly +the same moments and additionally identifies *which* overlay is focused. + +## Related + +- `FindReplaceOverlayCommandSupport`, `FindReplaceOverlayContextSupport` +- Commit `78c9a1c60a`, which replaced the shell-based overlay with the embedded composite +- Issue +- Issue , the workbench-part + proposal