+ * 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
+ * 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();
+ }
+
+ void fieldsLostFocus() {
+ if (!overlayContextActive) {
+ return;
+ }
+ overlayContextActive = false;
+ IEclipseContext toRestore = contextToRestore;
+ contextToRestore = null;
+ if (toRestore != null) {
+ handBackActiveLeaf(toRestore);
+ }
+ }
+
+ /**
+ * Gives the active leaf back to what held it before the overlay took over, but
+ * only while the overlay still holds it. Losing the focus to another part does
+ * not come through here as a hand-back: the workbench activates the part under
+ * the mouse before the focus leaves the field, so that part already owns the leaf
+ * by now and has to keep it.
+ *
+ * Handing it to the editor anyway would put the editor's key binding scope up
+ * beside the one the part coming up brings, and while both are up every key the
+ * two scopes have in common is an unresolvable binding conflict. Whether that
+ * surfaces depends on how the other part holds its scope, which is why it is
+ * observed with the console but not with an ordinary view.
+ *
+ * Activating the overlay's context replaced the window context's active child, so
+ * the whole chain down to the context to restore has to be re-established, not
+ * just its last link.
+ */
+ private void handBackActiveLeaf(IEclipseContext toRestore) {
+ if (overlayContext != null && overlayContext.getParent().getActiveLeaf() == overlayContext) {
+ toRestore.activateBranch();
+ }
+ }
+
+ void dispose() {
+ if (overlayContext != null) {
+ // The context is about to go, and disposing it while it holds the active leaf
+ // would leave the window context itself as the leaf, with the editor's scopes
+ // on no chain at all.
+ 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..323de78623b
--- /dev/null
+++ b/docs/adr/0001-find-replace-overlay-key-handling.md
@@ -0,0 +1,321 @@
+# 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
+
+- Nothing in the key handling depends on the editor type, and the overlay does not reference
+ `AbstractTextEditor`, as goal 4 requires.
+- 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.
+- Taking the editor out of the resolution path does not take the bindings of its commands with it:
+ those declared without a `contextId` live in the window scope, which stays active, so such a key
+ still resolves to its command and then finds nothing to run. Commands that remain meaningful
+ inside the overlay are therefore adopted, by activating a handler of the overlay's own for the
+ editor's command id and implementing the operation in terms of the overlay's state. Adopting the
+ id rather than the key sequence keeps this working when the user rebinds the command. Which
+ commands are adopted is decided one by one, since it only makes sense where the overlay has a
+ meaning of its own for the command.
+- Multi-page editors need no handling of their own, and the mechanism that used to provide it is
+ removed rather than replaced. `LegacyEditorActionBarExpression` compares `activePartId` against
+ the editor id it was constructed with and does nothing else, so shadowing that variable with the
+ overlay's id makes it false whichever editor id it carries and whichever of a multi-page editor's
+ two registration paths put the handler there. The gap that made the contributor variant fail, and
+ that `DeactivateGlobalActionHandlers` was introduced to close, therefore cannot arise here. This
+ follows from the expression's implementation; no test pins it, since the existing find/replace
+ tests do not exercise multi-page editors.
+- `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.
+- Handing the active leaf back when an input field loses focus must happen only while the overlay
+ still holds that leaf. A click activates the part under the mouse before the focus leaves the
+ field, so on that path the new part already owns the leaf and has to keep it. Handing it to the
+ editor anyway raises the editor's key-binding scope beside the one the new part brings, and while
+ both are up every stroke the two scopes share is an unresolvable binding conflict. Observed by
+ clicking from the overlay into the console, which reports conflicting zoom bindings; it is
+ reported from `PartServiceImpl#activate`, when the context updates deferred for the part switch
+ are flushed. Whether it surfaces depends on how the other part holds its scope: a scope that
+ rides the active-leaf chain drops as the leaf moves, which is why an ordinary view does not show
+ it. Clicking into the console without the overlay involved does not produce it either.
+
+## 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::