Fix issue 14489: ToolStrip (TabStop=true) intercepts arrow keys after tab navigation, preventing TreeView from handling them#14491
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR addresses a .NET 10/11 regression where a ToolStrip with TabStop=true can remain “active” in ModalMenuFilter after tab-navigation, causing arrow keys to be intercepted (e.g., preventing TreeView from handling them). The change narrows when a ToolStrip becomes active in menu mode and adds logic to clear the active ToolStrip when focus leaves ToolStrip context.
Changes:
- Gate
ModalMenuFilter.SetActiveToolStrip(this)inToolStrip.WndProc(WM_SETFOCUS)so it only runs for focus transitions originating from ToolStrip context (preserving #12916 behavior while avoiding #14489). - On
ToolStrip.OnLostFocus, remove the ToolStrip from the modal menu filter and potentially exit menu mode when focus moves outside ToolStrip context.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LeafShi1
left a comment
There was a problem hiding this comment.
Thanks for the fix, @SimonZhao888! The approach of gating SetActiveToolStrip on ToolStrip-context focus transitions and clearing on WM_KILLFOCUS makes sense. I have a few observations below.
| // Clear modal menu tracking when focus leaves ToolStrip context so | ||
| // keyboard input is not incorrectly routed after tabbing away. | ||
| HWND nextFocus = (HWND)(nint)m.WParamInternal; | ||
| Control? nextControl = FromHandle(nextFocus); |
There was a problem hiding this comment.
Inconsistent handle resolution: FromHandle vs FromChildHandle
The WM_SETFOCUS handler (line 4586) correctly uses FromChildHandle(previousFocus) to resolve the losing-focus HWND - this walks up and finds the owning WinForms Control even when the HWND belongs to a child/embedded window (e.g. the inner EDIT of a ToolStripComboBox).
However, this WM_KILLFOCUS handler uses FromHandle(nextFocus), which only returns a Control if the HWND is the exact top-level handle of a WinForms control. For child HWNDs it returns null.
Scenario: Focus moves from this ToolStrip to a ToolStripComboBox's inner edit field. FromHandle returns null, so nextControl is null, the is not ToolStrip / Parent is not ToolStrip checks both pass, and RemoveActiveToolStrip fires - incorrectly tearing down the modal menu filter even though focus is still within ToolStrip context.
Suggestion: Use FromChildHandle(nextFocus) here for consistency, matching the WM_SETFOCUS path.
| && nextControl is not ToolStrip | ||
| && nextControl?.Parent is not ToolStrip) | ||
| { | ||
| ToolStripManager.ModalMenuFilter.RemoveActiveToolStrip(this); |
There was a problem hiding this comment.
RemoveActiveToolStrip without ExitMenuMode may leave a zombie menu-mode state
RemoveActiveToolStripCore removes the ToolStrip from _inputFilterQueue and clears _toplevelToolStrip, but it does not call ExitMenuMode(). If this ToolStrip is the only entry in the queue, the queue becomes empty while _inMenuMode remains true and the application-level message filter stays installed with no active ToolStrip to route to.
Compare with CloseActiveDropdown() in ModalMenuFilter which explicitly checks if (GetActiveToolStrip() is null) { ExitMenuMode(); } after removing.
Suggestion: After calling RemoveActiveToolStrip(this), check whether there are remaining active tool strips and, if not, also call ExitMenuMode(). For example:
ToolStripManager.ModalMenuFilter.RemoveActiveToolStrip(this);
if (ToolStripManager.ModalMenuFilter.GetActiveToolStrip() is null)
{
ToolStripManager.ModalMenuFilter.ExitMenuMode();
}|
|
||
| await InputSimulator.SendAsync(form, VIRTUAL_KEY.VK_DOWN); | ||
| Assert.Equal(treeView.Nodes[1], treeView.SelectedNode); | ||
| }); |
There was a problem hiding this comment.
Consider adding a ToolStrip-to-ToolStrip regression test for #12916
Since this PR modifies the gating logic that was originally added to fix #12916 (ToolStrip-to-ToolStrip activation), it would be valuable to also add a test that verifies that scenario still works - e.g., two ToolStrips on a Form, Tab from one to the other, and assert the modal menu filter is (still) correctly activated. This protects against future regressions of #12916 when this code area is touched again.
Fixes #14489
Proposed changes
Customer Impact
Regression?
Risk
Screenshots
Before
ToolStripIssue.mp4
After
2026-04-24.141412.mp4
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow