Skip to content

Fix issue 14489: ToolStrip (TabStop=true) intercepts arrow keys after tab navigation, preventing TreeView from handling them#14491

Open
SimonZhao888 wants to merge 3 commits intodotnet:mainfrom
SimonZhao888:Fix_issue_14489
Open

Fix issue 14489: ToolStrip (TabStop=true) intercepts arrow keys after tab navigation, preventing TreeView from handling them#14491
SimonZhao888 wants to merge 3 commits intodotnet:mainfrom
SimonZhao888:Fix_issue_14489

Conversation

@SimonZhao888
Copy link
Copy Markdown
Member

@SimonZhao888 SimonZhao888 commented Apr 23, 2026

Fixes #14489

Proposed changes

Customer Impact

  • ToolStrip (TabStop=true) no longer intercepts arrow keys after tab navigation, and not preventing TreeView from handling them.

Regression?

  • Yes

Risk

  • minimal

Screenshots

Before

ToolStripIssue.mp4

After

2026-04-24.141412.mp4

Test methodology

  • Manually

Test environment(s)

  • .net 11.0.0-preview.2.final
Microsoft Reviewers: Open in CodeFlow

@SimonZhao888 SimonZhao888 changed the title Fix issue 14489 Fix issue 14489: ToolStrip (TabStop=true) intercepts arrow keys after tab navigation, preventing TreeView from handling them Apr 23, 2026
@dotnet-policy-service dotnet-policy-service Bot added the draft draft PR label Apr 23, 2026
@SimonZhao888 SimonZhao888 marked this pull request as ready for review April 24, 2026 06:17
@SimonZhao888 SimonZhao888 requested a review from a team as a code owner April 24, 2026 06:17
@SimonZhao888 SimonZhao888 requested a review from Copilot April 24, 2026 06:17
@SimonZhao888 SimonZhao888 removed the draft draft PR label Apr 24, 2026
@SimonZhao888
Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) in ToolStrip.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.

Comment thread src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStrip.cs Outdated
@SimonZhao888 SimonZhao888 added the waiting-review This item is waiting on review by one or more members of team label Apr 24, 2026
Copy link
Copy Markdown
Member

@LeafShi1 LeafShi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Infrastructure waiting-review This item is waiting on review by one or more members of team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Accessibility] ToolStrip (TabStop=true) intercepts arrow keys after tab navigation, preventing TreeView from handling them (.NET 10 regression)

3 participants