Fix commands being executed multiple times due to stale isEnabled() check

[HeikoKlare]

Problem

Since commit 35f5f09 ("Process KeyBinding Command Asyncly for Browser", fixes #3104), certain commands triggered via key bindings are executed twice. A prominent example is Paste (Ctrl+V) on a Combo widget, where text from the clipboard is inserted twice.

The duplicate execution follows two paths:

1. Via KeyBindingDispatcher → HandlerService → the registered handler (e.g. WidgetMethodHandler)
2. Via the OS native key event handler, which fires because Eclipse did not consume the key event

Root Cause

Change 1 — command.isEnabled() added to commandHandled evaluation

Commit 35f5f09 changed the commandHandled evaluation in executeCommand():

// before
commandHandled = ((IHandler) obj).isHandled();

// after (introduced by 35f5f09e)
commandHandled = command.isEnabled() && handler.isHandled();

commandHandled is what executeCommand() returns, and that return value determines whether processKeyEvent() sets event.doit = false to consume the key event.

command.isEnabled() delegates to handler.isEnabled(). For IHandler2 implementations, this reflects the result of the last setEnabled(evaluationContext) call — made during menu/toolbar refresh cycles, not at the point of the key event. The enabled state is therefore stale and does not reflect the currently active widget context.

By contrast, inside Command.executeWithChecks() (called from handlerService.executeHandler()), the framework calls setEnabled(event.getApplicationContext()) immediately before the same isEnabled() check, giving a fresh context-aware result. These two checks are not equivalent and do disagree for context-sensitive handlers like WidgetMethodHandler.

When command.isEnabled() incorrectly returns false:

• commandHandled is false
• executeCommand() returns false
• the key event is not consumed (event.doit remains true)
• the OS processes the keystroke natively, triggering a second execution

Change 2 — commandHandled = false on CommandException was removed

The original code set commandHandled = false if execution produced a CommandException (e.g. a NotEnabledException thrown from executeWithChecks() after a fresh setEnabled() evaluation). This ensured the key was not eaten when execution genuinely failed. The commit removed this line, leaving no mechanism to correct commandHandled based on the actual execution outcome for the synchronous path.

Solution

Fix 1 (critical) — revert commandHandled to use only handler.isHandled()

commandHandled = handler.isHandled();

handler.isHandled() answers the right question here: "is there a handler registered and capable of processing this command?" It does not depend on stale evaluation context. This is sufficient to decide whether Eclipse should claim ownership of the key event.

Why this is sound for the synchronous (non-Browser) path: execution happens immediately in the same call stack. If isHandled() is true but execution fails (see Fix 2), commandHandled is corrected before returning.

Why this is sound for the async (Browser) path: the key event must be consumed or not consumed synchronously, before the async execution runs. Basing that decision on handler.isHandled() is the right speculative choice — if we do not consume the key, the Browser's native handler will also process it, causing the exact duplicate-execution bug reported in #4080. The async path was introduced precisely to break a deadlock; eating the key speculatively is the correct trade-off.

Fix 2 (secondary) — restore commandHandled = false on CommandException for the synchronous path

handleCommandExecution() is changed to return a boolean (false if a CommandException was recorded in the staticContext after execution, true otherwise). The synchronous branch of executeCommand() uses this return value to update commandHandled before returning:

if (trigger != null && trigger.widget instanceof Browser) {
    getDisplay().asyncExec(() -> handleCommandExecution(...));
} else {
    commandHandled = handleCommandExecution(...) && commandHandled;
}

This restores the original correctness property: a handler that claims isHandled() but fails during execution (because executeWithChecks() finds it disabled after a fresh setEnabled() call) does not eat the key event.

The async Browser path remains void — a return value is not available before the caller returns, and the speculative eat-the-key decision based on handler.isHandled() is correct by the reasoning above.

Fixes #4080

[eclipse-platform-bot]

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From a76a4d78f4cd14566d63224dcd951d285b027c5c Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Mon, 15 Jun 2026 18:08:23 +0000
Subject: [PATCH] Version bump(s) for 4.41 stream


diff --git a/bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF
index a30c1d6c35..84b518f63d 100644
--- a/bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.bindings;singleton:=true
-Bundle-Version: 0.15.100.qualifier
+Bundle-Version: 0.15.200.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
-- 
2.54.0

Further information are available in Common Build Issues - Missing version increments.

[github-actions]

Test Results

   864 files  ±0     864 suites  ±0   1h 0m 16s ⏱️ + 15m 24s
 8 051 tests +1   7 808 ✅ +1  243 💤 ±0  0 ❌ ±0 
20 592 runs  +8  19 937 ✅ +8  655 💤 ±0  0 ❌ ±0 

Results for commit 96a0915. ± Comparison against base commit 4f9ee7a.

♻️ This comment has been updated with latest results.