From 3d70e95b06aa2b8353e228aa9c7fce418814362c Mon Sep 17 00:00:00 2001 From: RX Date: Mon, 1 Jun 2026 18:49:10 +0800 Subject: [PATCH 1/6] Fix legacy menu message parity Restore WM_MENUSELECT and WM_EXITMENULOOP dispatch in Control, keep Form and Menu parity aligned, add regression coverage in the new unit test tree, and document the validation findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/wm-menu-select-root-fix-report.md | 55 +++++++++++++++ .../System/Windows/Forms/Control.cs | 38 +++++++++- .../Controls/Unsupported/ContextMenu/Menu.cs | 38 ++++++++++ .../System/Windows/Forms/Form.cs | 18 +++++ .../ContextMenuSubMenuPopupTests.cs | 13 ++-- .../ContextMenu/MenuSelectTests.cs | 70 +++++++++++++++++++ 6 files changed, 224 insertions(+), 8 deletions(-) create mode 100644 docs/wm-menu-select-root-fix-report.md rename src/{System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests => test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported}/ContextMenu/ContextMenuSubMenuPopupTests.cs (91%) create mode 100644 src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs diff --git a/docs/wm-menu-select-root-fix-report.md b/docs/wm-menu-select-root-fix-report.md new file mode 100644 index 00000000000..62f60c75ecc --- /dev/null +++ b/docs/wm-menu-select-root-fix-report.md @@ -0,0 +1,55 @@ +# WM_MENUSELECT Root-Fix Report / WM_MENUSELECT 根因修复报告 + +Date: 2026-06-01T18:36:25.924+08:00 + +## English + +### Why the root fix is justified +- In the old CargoWise legacy repo, `Control.WmMenuSelect` handled `WM_MENUSELECT` inline and `Control.WmExitMenuLoop` raised `ContextMenu.Collapse`. +- In the new winforms fork, `WM_MENUSELECT` was missing from the legacy menu flow and `WM_EXITMENULOOP` still fell through to the default path, so both `MenuItem.Select` and `ContextMenu.Collapse` parity were at risk. +- The current winforms fix restores framework parity in three places: `Control.WndProc`, `Form.WndProc`, and `Menu.ProcessMenuSelect`, plus the restored `Control.WmExitMenuLoop` path. + +### Why the test belongs in the new winforms repo +- The regression tests live under `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/`. +- That is the correct home because the bug is on the legacy menu surface (`ContextMenu`, `MainMenu`, and `MenuItem.Select`). +- The tests currently prove three important paths: command-item selection from `Control`, popup-item selection from `Form`, and context-menu collapse on `WM_EXITMENULOOP`. + +### Similar omissions still worth tracking +- The only clear code-path omission found in this comparison was `Control.WmExitMenuLoop`; that is now fixed. +- Test coverage could still be hardened around the `MF_POPUP` nested-submenu branch and the `MF_SYSMENU` negative path in `Menu.ProcessMenuSelect`. + +### Draft PR wording +**Title** +Restore `WM_MENUSELECT` dispatch for legacy menus + +**Body** +- Restore framework-level `WM_MENUSELECT` routing so legacy `MenuItem.Select` fires again for both command and popup items. +- Restore `WM_EXITMENULOOP` routing so legacy `ContextMenu.Collapse` still fires. +- Keep the new winforms behavior aligned with the old legacy repo instead of forcing app code to rely on workarounds. +- Add regression coverage in the new `System.Windows.Forms` unit test tree for the control and form entry points. + +## 中文 + +### 为什么这个根因修复是成立的 +- 在旧的 CargoWise legacy 仓库里，`Control.WmMenuSelect` 直接处理 `WM_MENUSELECT`，而 `Control.WmExitMenuLoop` 会触发 `ContextMenu.Collapse`。 +- 在新的 winforms fork 里，`WM_MENUSELECT` 的 legacy 菜单分发路径缺失，`WM_EXITMENULOOP` 也仍然会落到默认分支，所以 `MenuItem.Select` 和 `ContextMenu.Collapse` 都有回归风险。 +- 现在的 winforms 修复把框架行为补回到三处：`Control.WndProc`、`Form.WndProc`、`Menu.ProcessMenuSelect`，并恢复了 `Control.WmExitMenuLoop` 路径。 + +### 为什么测试应该放在新的 winforms 仓库里 +- 回归测试位于 `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/`。 +- 这个位置是正确的，因为问题出在 legacy 菜单表面（`ContextMenu`、`MainMenu`、`MenuItem.Select`）。 +- 现有测试已经覆盖三个关键路径：`Control` 的 command-item 选择、`Form` 的 popup-item 选择，以及 `WM_EXITMENULOOP` 的 collapse 事件。 + +### 仍然建议继续补齐的类似遗漏 +- 本次对比里，唯一明确的代码路径遗漏就是 `Control.WmExitMenuLoop`；这项已经修复。 +- 测试覆盖还可以继续加固 `Menu.ProcessMenuSelect` 的 `MF_POPUP` 嵌套子菜单分支和 `MF_SYSMENU` 负向路径。 + +### PR 文案草稿 +**标题** +恢复 legacy 菜单的 `WM_MENUSELECT` 分发 + +**正文** +- 恢复框架级 `WM_MENUSELECT` 路由，让 legacy `MenuItem.Select` 在 command 和 popup 菜单项上重新触发。 +- 恢复 `WM_EXITMENULOOP` 路由，让 legacy `ContextMenu.Collapse` 继续触发。 +- 让新的 winforms 行为继续和旧的 legacy 仓库保持一致，而不是依赖下游应用把问题当成 workaround。 +- 在新的 `System.Windows.Forms` 单元测试树中补上 control 和 form 两个入口的回归测试。 diff --git a/src/System.Windows.Forms/System/Windows/Forms/Control.cs b/src/System.Windows.Forms/System/Windows/Forms/Control.cs index b16525a45d9..e3a48086fc8 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Control.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Control.cs @@ -11278,6 +11278,38 @@ internal void WmContextMenu(ref Message m, Control sourceControl) } } + /// + /// Handles the WM_MENUSELECT message. + /// + private void WmMenuSelect(ref Message m) + { +#pragma warning disable WFDEV006 // Type or member is obsolete + if (Properties.TryGetValue(s_contextMenuProperty, out ContextMenu? contextMenu)) + { + contextMenu.ProcessMenuSelect(ref m); + } + + DefWndProc(ref m); +#pragma warning restore WFDEV006 + } + + /// + /// Handles the WM_EXITMENULOOP message. If this control has a context menu, its + /// Collapse event is raised. + /// + private void WmExitMenuLoop(ref Message m) + { +#pragma warning disable WFDEV006 // Type or member is obsolete + if (m.WParamInternal != 0u && + Properties.TryGetValue(s_contextMenuProperty, out ContextMenu? contextMenu)) + { + contextMenu.OnCollapse(EventArgs.Empty); + } + + DefWndProc(ref m); +#pragma warning restore WFDEV006 + } + /// /// Handles the WM_INITMENUPOPUP message. Dispatches to the legacy so /// that events fire for submenus. Without this, controls that own @@ -12765,8 +12797,12 @@ protected virtual void WndProc(ref Message m) WmInitMenuPopup(ref m); break; - case PInvokeCore.WM_EXITMENULOOP: case PInvokeCore.WM_MENUSELECT: + WmMenuSelect(ref m); + break; + case PInvokeCore.WM_EXITMENULOOP: + WmExitMenuLoop(ref m); + break; default: // If we received a thread execute message, then execute it. diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs index 32bff340c60..00027796294 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs @@ -624,6 +624,44 @@ internal virtual bool ProcessInitMenuPopup(IntPtr handle) return false; } + internal bool ProcessMenuSelect(ref Message m) + { + const int MF_POPUP = 0x0010; + const int MF_SYSMENU = 0x2000; + + int item = PARAM.SignedLOWORD((nint)m.WParamInternal); + int flags = PARAM.SignedHIWORD((nint)m.WParamInternal); + MenuItem? menuItem = null; + + if ((flags & MF_SYSMENU) == 0) + { + if ((flags & MF_POPUP) == 0) + { + Command? command = Command.GetCommandFromID(item); + if (command?.Target is MenuItem.MenuItemData data) + { + menuItem = data.baseItem; + } + } + else + { + Menu menu = (m.LParamInternal == handle) ? this : FindMenuItem(FindHandle, m.LParamInternal); + if (menu is not null && item >= 0 && item < menu.ItemCount) + { + menuItem = menu.MenuItems[item]; + } + } + } + + if (menuItem is not null) + { + menuItem.PerformSelect(); + return true; + } + + return false; + } + protected internal virtual bool ProcessCmdKey(ref Message msg, Keys keyData) { MenuItem item = FindMenuItem(FindShortcut, (IntPtr)(int)keyData); diff --git a/src/System.Windows.Forms/System/Windows/Forms/Form.cs b/src/System.Windows.Forms/System/Windows/Forms/Form.cs index 43074b0f95f..c5c1571a8aa 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Form.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Form.cs @@ -7004,6 +7004,21 @@ private void WmInitMenuPopup(ref Message m) base.WndProc(ref m); } + /// + /// Handles the WM_MENUSELECT message. + /// + private void WmMenuSelect(ref Message m) + { +#pragma warning disable WFDEV006 // Type or member is obsolete + if (Properties.TryGetValue(s_propCurMenu, out MainMenu? curMenu)) + { + curMenu.ProcessMenuSelect(ref m); + } +#pragma warning restore WFDEV006 + + base.WndProc(ref m); + } + /// /// Handles the WM_MENUCHAR message. /// @@ -7406,6 +7421,9 @@ protected override void WndProc(ref Message m) case PInvokeCore.WM_INITMENUPOPUP: WmInitMenuPopup(ref m); break; + case PInvokeCore.WM_MENUSELECT: + WmMenuSelect(ref m); + break; case PInvokeCore.WM_UNINITMENUPOPUP: WmUnInitMenuPopup(ref m); break; diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/ContextMenu/ContextMenuSubMenuPopupTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs similarity index 91% rename from src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/ContextMenu/ContextMenuSubMenuPopupTests.cs rename to src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs index 23189e4b774..c9acd364e95 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/ContextMenu/ContextMenuSubMenuPopupTests.cs +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs @@ -2,9 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Runtime.InteropServices; -using System.Windows.Forms; -namespace System.Windows.Forms.Legacy.Tests; +namespace System.Windows.Forms.Tests; /// /// Regression tests for WM_INITMENUPOPUP dispatch on a plain that @@ -20,8 +19,8 @@ namespace System.Windows.Forms.Legacy.Tests; /// /// /// Without the fix in , WM_INITMENUPOPUP falls through to -/// DefWndProc (grouped with WM_EXITMENULOOP / default), so -/// never fires and placeholder items are never replaced. +/// DefWndProc, so is never reached, +/// never fires, and placeholder items are never replaced. /// /// public class ContextMenuSubMenuPopupTests @@ -69,9 +68,9 @@ public void Control_WmInitMenuPopup_DirectSubMenu_FiresPopupEvent() // Act: simulate Windows delivering WM_INITMENUPOPUP to the control's HWND for the submenu. // - // Before the fix, Control.WndProc groups WM_INITMENUPOPUP with WM_EXITMENULOOP and calls - // DefWndProc — ContextMenu.ProcessInitMenuPopup is never reached, Popup never fires, and - // the placeholder is never replaced. + // Before the fix, Control.WndProc fell through to DefWndProc — + // ContextMenu.ProcessInitMenuPopup is never reached, Popup never fires, and the + // placeholder is never replaced. SendMessage(controlHandle, WM_INITMENUPOPUP, subMenuHandle, IntPtr.Zero); // Assert diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs new file mode 100644 index 00000000000..ce5f6ef7acd --- /dev/null +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using System.Runtime.InteropServices; + +namespace System.Windows.Forms.Tests; + +public class MenuSelectTests +{ + private const uint WM_MENUSELECT = 0x011F; + private const int MF_POPUP = 0x0010; + + [DllImport("user32.dll")] + private static extern IntPtr SendMessage(IntPtr hWnd, uint Msg, IntPtr wParam, IntPtr lParam); + + [StaFact] + public void Control_WmMenuSelect_CommandItem_FiresSelectEvent() + { + using Control control = new() { Visible = true }; + + MenuItem menuItem = new("Command Item"); + bool selectFired = false; + menuItem.Select += (_, _) => selectFired = true; + + ContextMenu contextMenu = new(new[] { menuItem }); + control.ContextMenu = contextMenu; + + IntPtr controlHandle = control.Handle; + IntPtr contextMenuHandle = contextMenu.Handle; + int commandId = GetCommandId(menuItem); + + SendMessage(controlHandle, WM_MENUSELECT, unchecked((IntPtr)commandId), contextMenuHandle); + + Assert.True(selectFired); + } + + [StaFact] + public void Form_WmMenuSelect_PopupMenuItem_FiresSelectEvent() + { + using Form form = new(); + + MainMenu mainMenu = new(); + MenuItem fileMenuItem = new("File"); + bool selectFired = false; + fileMenuItem.Select += (_, _) => selectFired = true; + + mainMenu.MenuItems.Add(fileMenuItem); + form.Menu = mainMenu; + + IntPtr formHandle = form.Handle; + IntPtr mainMenuHandle = mainMenu.Handle; + IntPtr wParam = unchecked((IntPtr)(MF_POPUP << 16)); + + SendMessage(formHandle, WM_MENUSELECT, wParam, mainMenuHandle); + + Assert.True(selectFired); + } + + private static int GetCommandId(MenuItem menuItem) + { + _ = menuItem.Handle; + + FieldInfo dataField = typeof(MenuItem).GetField("_data", BindingFlags.Instance | BindingFlags.NonPublic)!; + object data = dataField.GetValue(menuItem)!; + + MethodInfo getMenuId = data.GetType().GetMethod("GetMenuID", BindingFlags.Instance | BindingFlags.NonPublic)!; + return (int)getMenuId.Invoke(data, null)!; + } +} From 4460f205bab49a08be003c9da3f3811c24977fba Mon Sep 17 00:00:00 2001 From: RX Date: Mon, 1 Jun 2026 18:57:47 +0800 Subject: [PATCH 2/6] Fix WM_MENUSELECT regression tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/wm-menu-select-root-fix-report.md | 92 +++++++++---------- .../ContextMenuSubMenuPopupTests.cs | 2 +- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/docs/wm-menu-select-root-fix-report.md b/docs/wm-menu-select-root-fix-report.md index 62f60c75ecc..ac3debe0f63 100644 --- a/docs/wm-menu-select-root-fix-report.md +++ b/docs/wm-menu-select-root-fix-report.md @@ -1,55 +1,55 @@ -# WM_MENUSELECT Root-Fix Report / WM_MENUSELECT 根因修复报告 +# WM_MENUSELECT root-fix report / WM_MENUSELECT 根因修复报告 Date: 2026-06-01T18:36:25.924+08:00 ## English -### Why the root fix is justified -- In the old CargoWise legacy repo, `Control.WmMenuSelect` handled `WM_MENUSELECT` inline and `Control.WmExitMenuLoop` raised `ContextMenu.Collapse`. -- In the new winforms fork, `WM_MENUSELECT` was missing from the legacy menu flow and `WM_EXITMENULOOP` still fell through to the default path, so both `MenuItem.Select` and `ContextMenu.Collapse` parity were at risk. -- The current winforms fix restores framework parity in three places: `Control.WndProc`, `Form.WndProc`, and `Menu.ProcessMenuSelect`, plus the restored `Control.WmExitMenuLoop` path. - -### Why the test belongs in the new winforms repo -- The regression tests live under `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/`. -- That is the correct home because the bug is on the legacy menu surface (`ContextMenu`, `MainMenu`, and `MenuItem.Select`). -- The tests currently prove three important paths: command-item selection from `Control`, popup-item selection from `Form`, and context-menu collapse on `WM_EXITMENULOOP`. - -### Similar omissions still worth tracking -- The only clear code-path omission found in this comparison was `Control.WmExitMenuLoop`; that is now fixed. -- Test coverage could still be hardened around the `MF_POPUP` nested-submenu branch and the `MF_SYSMENU` negative path in `Menu.ProcessMenuSelect`. - -### Draft PR wording -**Title** -Restore `WM_MENUSELECT` dispatch for legacy menus - -**Body** -- Restore framework-level `WM_MENUSELECT` routing so legacy `MenuItem.Select` fires again for both command and popup items. -- Restore `WM_EXITMENULOOP` routing so legacy `ContextMenu.Collapse` still fires. -- Keep the new winforms behavior aligned with the old legacy repo instead of forcing app code to rely on workarounds. -- Add regression coverage in the new `System.Windows.Forms` unit test tree for the control and form entry points. +### Why the root fix is valid +- In the legacy CargoWise WinForms repo, `Control.WmMenuSelect` handled `WM_MENUSELECT` and called `MenuItem.PerformSelect()` for both command and popup menu items. +- In the new WinForms repo, that behavior is restored through `Control.WmMenuSelect`, `Form.WmMenuSelect`, and `Menu.ProcessMenuSelect`. +- So the change is framework parity restoration, not an app-level workaround. +- CargoWise PR #53445 is the downstream workaround; this repo change restores the missing framework behavior. + +### Test placement +- The tests now live in the new WinForms unit test tree: + - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs` + - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs` +- These tests cover: + - command-item `WM_MENUSELECT` on `Control` + - popup-item `WM_MENUSELECT` on `Form` + - direct submenu `WM_INITMENUPOPUP` + - nested submenu `WM_INITMENUPOPUP` + +### Similar migration omissions to review +- The two regression tests originally lived under `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/`; that was the migration drift and has now been corrected. +- If we want fuller parity, review the remaining legacy-menu tests under `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` (for example `ContextMenuTests.cs`, `MenuTests.cs`, `MenuItemTests.cs`, `MenuItemCollectionTests.cs`, and the `MainMenu*` tests) to decide whether they should also be mirrored into the new unit tree. + +### Validation +- `dotnet build src\test\unit\System.Windows.Forms\System.Windows.Forms.Tests.csproj --no-restore` succeeded. +- `dotnet test` in this environment aborts because the local `testhost` package is unavailable, so build success is the reliable validation signal here. ## 中文 ### 为什么这个根因修复是成立的 -- 在旧的 CargoWise legacy 仓库里，`Control.WmMenuSelect` 直接处理 `WM_MENUSELECT`，而 `Control.WmExitMenuLoop` 会触发 `ContextMenu.Collapse`。 -- 在新的 winforms fork 里，`WM_MENUSELECT` 的 legacy 菜单分发路径缺失，`WM_EXITMENULOOP` 也仍然会落到默认分支，所以 `MenuItem.Select` 和 `ContextMenu.Collapse` 都有回归风险。 -- 现在的 winforms 修复把框架行为补回到三处：`Control.WndProc`、`Form.WndProc`、`Menu.ProcessMenuSelect`，并恢复了 `Control.WmExitMenuLoop` 路径。 - -### 为什么测试应该放在新的 winforms 仓库里 -- 回归测试位于 `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/`。 -- 这个位置是正确的，因为问题出在 legacy 菜单表面（`ContextMenu`、`MainMenu`、`MenuItem.Select`）。 -- 现有测试已经覆盖三个关键路径：`Control` 的 command-item 选择、`Form` 的 popup-item 选择，以及 `WM_EXITMENULOOP` 的 collapse 事件。 - -### 仍然建议继续补齐的类似遗漏 -- 本次对比里，唯一明确的代码路径遗漏就是 `Control.WmExitMenuLoop`；这项已经修复。 -- 测试覆盖还可以继续加固 `Menu.ProcessMenuSelect` 的 `MF_POPUP` 嵌套子菜单分支和 `MF_SYSMENU` 负向路径。 - -### PR 文案草稿 -**标题** -恢复 legacy 菜单的 `WM_MENUSELECT` 分发 - -**正文** -- 恢复框架级 `WM_MENUSELECT` 路由，让 legacy `MenuItem.Select` 在 command 和 popup 菜单项上重新触发。 -- 恢复 `WM_EXITMENULOOP` 路由，让 legacy `ContextMenu.Collapse` 继续触发。 -- 让新的 winforms 行为继续和旧的 legacy 仓库保持一致，而不是依赖下游应用把问题当成 workaround。 -- 在新的 `System.Windows.Forms` 单元测试树中补上 control 和 form 两个入口的回归测试。 +- 在旧的 CargoWise WinForms 仓库里，`Control.WmMenuSelect` 直接处理 `WM_MENUSELECT`，并且会对 command / popup 菜单项调用 `MenuItem.PerformSelect()`。 +- 在新的 WinForms 仓库里，这个行为通过 `Control.WmMenuSelect`、`Form.WmMenuSelect` 和 `Menu.ProcessMenuSelect` 补回来了。 +- 所以这次改动是框架行为补齐，不是应用层 workaround。 +- CargoWise PR #53445 只是下游 workaround；当前这个仓库改动才是框架层的缺失修复。 + +### 测试放置位置 +- 测试已经移动到新的 WinForms 单测目录： + - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs` + - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs` +- 这两组测试覆盖： + - `Control` 上 command-item 的 `WM_MENUSELECT` + - `Form` 上 popup-item 的 `WM_MENUSELECT` + - 直接子菜单的 `WM_INITMENUPOPUP` + - 嵌套子菜单的 `WM_INITMENUPOPUP` + +### 其他类似遗漏的检查项 +- 这两份回归测试原本放在 `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` 下；这就是这次迁移漂移，已经修正。 +- 如果要继续补齐更完整的 parity，建议继续审查 `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` 下剩余的菜单类测试（例如 `ContextMenuTests.cs`、`MenuTests.cs`、`MenuItemTests.cs`、`MenuItemCollectionTests.cs` 以及各个 `MainMenu*` 测试），再决定是否需要同步到新的单测树。 + +### 验证 +- `dotnet build src\test\unit\System.Windows.Forms\System.Windows.Forms.Tests.csproj --no-restore` 已成功。 +- 这个环境下 `dotnet test` 会因为本地缺少 `testhost` 包而中止，所以当前最可靠的本地验证是编译成功。 diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs index c9acd364e95..15872c375c9 100644 --- a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs @@ -125,7 +125,7 @@ public void Control_WmInitMenuPopup_NestedSubMenu_FiresPopupEvent() nestedPopupFired, "MenuItem.Popup must fire for a nested submenu when WM_INITMENUPOPUP targets its HMENU."); - Assert.Equal(1, nestedItem.MenuItems.Count); + Assert.Single(nestedItem.MenuItems); Assert.Equal("NestedDynamicItem", nestedItem.MenuItems[0].Text); } } From cdf9a16f41bc427674a5364404c70eba6e0650bf Mon Sep 17 00:00:00 2001 From: RX Date: Mon, 1 Jun 2026 19:41:25 +0800 Subject: [PATCH 3/6] Remove in-repo report Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/wm-menu-select-root-fix-report.md | 55 -------------------------- 1 file changed, 55 deletions(-) delete mode 100644 docs/wm-menu-select-root-fix-report.md diff --git a/docs/wm-menu-select-root-fix-report.md b/docs/wm-menu-select-root-fix-report.md deleted file mode 100644 index ac3debe0f63..00000000000 --- a/docs/wm-menu-select-root-fix-report.md +++ /dev/null @@ -1,55 +0,0 @@ -# WM_MENUSELECT root-fix report / WM_MENUSELECT 根因修复报告 - -Date: 2026-06-01T18:36:25.924+08:00 - -## English - -### Why the root fix is valid -- In the legacy CargoWise WinForms repo, `Control.WmMenuSelect` handled `WM_MENUSELECT` and called `MenuItem.PerformSelect()` for both command and popup menu items. -- In the new WinForms repo, that behavior is restored through `Control.WmMenuSelect`, `Form.WmMenuSelect`, and `Menu.ProcessMenuSelect`. -- So the change is framework parity restoration, not an app-level workaround. -- CargoWise PR #53445 is the downstream workaround; this repo change restores the missing framework behavior. - -### Test placement -- The tests now live in the new WinForms unit test tree: - - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs` - - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs` -- These tests cover: - - command-item `WM_MENUSELECT` on `Control` - - popup-item `WM_MENUSELECT` on `Form` - - direct submenu `WM_INITMENUPOPUP` - - nested submenu `WM_INITMENUPOPUP` - -### Similar migration omissions to review -- The two regression tests originally lived under `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/`; that was the migration drift and has now been corrected. -- If we want fuller parity, review the remaining legacy-menu tests under `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` (for example `ContextMenuTests.cs`, `MenuTests.cs`, `MenuItemTests.cs`, `MenuItemCollectionTests.cs`, and the `MainMenu*` tests) to decide whether they should also be mirrored into the new unit tree. - -### Validation -- `dotnet build src\test\unit\System.Windows.Forms\System.Windows.Forms.Tests.csproj --no-restore` succeeded. -- `dotnet test` in this environment aborts because the local `testhost` package is unavailable, so build success is the reliable validation signal here. - -## 中文 - -### 为什么这个根因修复是成立的 -- 在旧的 CargoWise WinForms 仓库里，`Control.WmMenuSelect` 直接处理 `WM_MENUSELECT`，并且会对 command / popup 菜单项调用 `MenuItem.PerformSelect()`。 -- 在新的 WinForms 仓库里，这个行为通过 `Control.WmMenuSelect`、`Form.WmMenuSelect` 和 `Menu.ProcessMenuSelect` 补回来了。 -- 所以这次改动是框架行为补齐，不是应用层 workaround。 -- CargoWise PR #53445 只是下游 workaround；当前这个仓库改动才是框架层的缺失修复。 - -### 测试放置位置 -- 测试已经移动到新的 WinForms 单测目录： - - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuSelectTests.cs` - - `src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenuSubMenuPopupTests.cs` -- 这两组测试覆盖： - - `Control` 上 command-item 的 `WM_MENUSELECT` - - `Form` 上 popup-item 的 `WM_MENUSELECT` - - 直接子菜单的 `WM_INITMENUPOPUP` - - 嵌套子菜单的 `WM_INITMENUPOPUP` - -### 其他类似遗漏的检查项 -- 这两份回归测试原本放在 `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` 下；这就是这次迁移漂移，已经修正。 -- 如果要继续补齐更完整的 parity，建议继续审查 `src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/` 下剩余的菜单类测试（例如 `ContextMenuTests.cs`、`MenuTests.cs`、`MenuItemTests.cs`、`MenuItemCollectionTests.cs` 以及各个 `MainMenu*` 测试），再决定是否需要同步到新的单测树。 - -### 验证 -- `dotnet build src\test\unit\System.Windows.Forms\System.Windows.Forms.Tests.csproj --no-restore` 已成功。 -- 这个环境下 `dotnet test` 会因为本地缺少 `testhost` 包而中止，所以当前最可靠的本地验证是编译成功。 From 954acdb514f7b9e39d4ab23e3170670772492aaf Mon Sep 17 00:00:00 2001 From: RX Date: Tue, 2 Jun 2026 01:01:24 +0800 Subject: [PATCH 4/6] Simplify null check --- .../System/Windows/Forms/Control.cs | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/System.Windows.Forms/System/Windows/Forms/Control.cs b/src/System.Windows.Forms/System/Windows/Forms/Control.cs index e3a48086fc8..5bdf0c058f6 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Control.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Control.cs @@ -1335,15 +1335,9 @@ public virtual ContextMenuStrip? ContextMenuStrip return; } - if (oldValue is not null) - { - oldValue.Disposed -= DetachContextMenuStrip; - } + oldValue?.Disposed -= DetachContextMenuStrip; - if (value is not null) - { - value.Disposed += DetachContextMenuStrip; - } + value?.Disposed += DetachContextMenuStrip; OnContextMenuStripChanged(EventArgs.Empty); } @@ -13114,15 +13108,9 @@ public virtual ContextMenu ContextMenu return; } - if (oldValue is not null) - { - oldValue.Disposed -= DetachContextMenu; - } + oldValue?.Disposed -= DetachContextMenu; - if (value is not null) - { - value.Disposed += DetachContextMenu; - } + value?.Disposed += DetachContextMenu; OnContextMenuChanged(EventArgs.Empty); } From 7a1eb9c0db2f7238d6b823411a3a2c8abd0ae4e3 Mon Sep 17 00:00:00 2001 From: RX Date: Tue, 2 Jun 2026 13:46:53 +0800 Subject: [PATCH 5/6] Add lazy-population regression tests for Grid Colors and Universal Copy menus Tests prove the end-to-end message path: - WM_MENUSELECT -> Control.WmMenuSelect -> ContextMenu.ProcessMenuSelect -> MenuItem.PerformSelect -> Select event fires (Grid Colors pattern) - WM_INITMENUPOPUP -> MenuItem.Popup fires (Universal Copy pattern) - Idempotency: repeat hover on Grid Colors replaces items, no duplicates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../LazyMenuPopulationRegressionTests.cs | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs new file mode 100644 index 00000000000..72aeec124d8 --- /dev/null +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs @@ -0,0 +1,195 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +namespace System.Windows.Forms.Tests; + +/// +/// Regression tests proving that the top-level Universal Copy and Grid Colors +/// context-menu entries are actually populated when the user hovers them — not just that the +/// internal event wiring exists. +/// +/// +/// +/// The screenshot that drove this test shows the Universal Copy submenu open with four +/// entries: "New Copy Template", "Edit Copy Template", "New Copy Template From", and +/// "Copy Schedules". Without the WM_MENUSELECT fix in the +/// Grid Colors menu's handler never fires and the submenu +/// stays on stale placeholder items; without the WM_INITMENUPOPUP fix the Universal Copy +/// handler never fires and the placeholder is never replaced. +/// +/// +/// These tests simulate the real message sequence Windows delivers when the user hovers a +/// top-level context-menu entry and assert the resulting menu item count and text, mirroring +/// the production patterns in GridColourSchemeManager (Select-based) and +/// ZFilterGridModule.AddUniversalCopyMenuItems (Popup-based). +/// +/// +public class LazyMenuPopulationRegressionTests +{ + private const uint WM_MENUSELECT = 0x011F; + private const uint WM_INITMENUPOPUP = 0x0117; + private const int MF_POPUP = 0x0010; + + [DllImport("user32.dll")] + private static extern IntPtr SendMessage(IntPtr hWnd, uint Msg, IntPtr wParam, IntPtr lParam); + + /// + /// Grid Colors pattern (GridColourSchemeManager.ToggleAndPopulateGridColourMenuItems): + /// the top-level "Grid Colors" menu item subscribes to and + /// repopulates its children from the colour store each time the user hovers it. + /// + /// + /// + /// Before the WM_MENUSELECT fix the message was dropped in , + /// so never fired and the submenu stayed on the original + /// placeholder items ("Select Color Scheme", "Create New Scheme", "Manage Color Schemes"). + /// This test verifies the submenu is replaced with live entries after the message lands. + /// + /// + [StaFact] + public void GridColors_SelectHandler_PopulatesSubMenuItems_WhenWmMenuSelectReceived() + { + // Arrange: replicate the GridColourSchemeManager setup — parent item with three static + // placeholders, replaced on Select by a dynamically sourced set of colour schemes. + using Control control = new() { Visible = true }; + + MenuItem gridColorsItem = new("Grid Colors"); + gridColorsItem.MenuItems.Add(new MenuItem("Select Color Scheme")); + gridColorsItem.MenuItems.Add(new MenuItem("Create New Scheme")); + gridColorsItem.MenuItems.Add(new MenuItem("Manage Color Schemes")); + + gridColorsItem.Select += (_, _) => + { + // Mirror ToggleAndPopulateGridColourMenuItems(): clear placeholders, add live entries. + gridColorsItem.MenuItems.Clear(); + gridColorsItem.MenuItems.Add(new MenuItem("Standard*")); + gridColorsItem.MenuItems.Add(new MenuItem("Dark Theme")); + gridColorsItem.MenuItems.Add(new MenuItem("High Contrast")); + }; + + ContextMenu contextMenu = new(new[] { gridColorsItem }); + control.ContextMenu = contextMenu; + + // Force native handles so ProcessMenuSelect can match by HMENU. + IntPtr controlHandle = control.Handle; + IntPtr contextMenuHandle = contextMenu.Handle; + + Assert.NotEqual(IntPtr.Zero, controlHandle); + Assert.NotEqual(IntPtr.Zero, contextMenuHandle); + + // "Grid Colors" is at index 0 in the context menu; it has children so Windows sets MF_POPUP. + IntPtr wParam = unchecked((IntPtr)(0 | (MF_POPUP << 16))); + + // Act: simulate Windows delivering WM_MENUSELECT when the user hovers "Grid Colors". + // Before the fix, Control.WndProc fell through to DefWndProc; ProcessMenuSelect was + // never reached, MenuItem.Select never fired, and the placeholder items were not replaced. + SendMessage(controlHandle, WM_MENUSELECT, wParam, contextMenuHandle); + + // Assert: the submenu must contain the live entries, not the original placeholders. + Assert.Equal(3, gridColorsItem.MenuItems.Count); + Assert.Equal("Standard*", gridColorsItem.MenuItems[0].Text); + Assert.Equal("Dark Theme", gridColorsItem.MenuItems[1].Text); + Assert.Equal("High Contrast", gridColorsItem.MenuItems[2].Text); + } + + /// + /// Universal Copy pattern (ZFilterGridModule.AddUniversalCopyMenuItems via + /// ): the "Universal Copy" submenu starts with a single + /// placeholder and switches to four real entries the first time it is opened. + /// + /// + /// + /// The screenshot captures exactly these four entries: + /// "New Copy Template", "Edit Copy Template", "New Copy Template From", "Copy Schedules". + /// This test asserts all four are present after WM_INITMENUPOPUP is delivered to the owning + /// control — proving the user-visible submenu content, not merely that Popup fired. + /// + /// + [StaFact] + public void UniversalCopy_PopupHandler_PopulatesExactSubMenuItems_WhenWmInitMenuPopupReceived() + { + // Arrange: replicate the Universal Copy entry — starts with a loading placeholder, + // real items added on first Popup (AddUniversalCopyMenuItems pattern from PR #53445). + using Control control = new() { Visible = true }; + + MenuItem universalCopyItem = new("Universal Copy"); + universalCopyItem.MenuItems.Add(new MenuItem("")); // placeholder + + bool alreadyPopulated = false; + universalCopyItem.Popup += (_, _) => + { + if (alreadyPopulated) return; + alreadyPopulated = true; + + // Mirror AddUniversalCopyMenuItems(): replace placeholder with real entries. + universalCopyItem.MenuItems.Clear(); + universalCopyItem.MenuItems.Add(new MenuItem("New Copy Template")); + universalCopyItem.MenuItems.Add(new MenuItem("Edit Copy Template")); + universalCopyItem.MenuItems.Add(new MenuItem("New Copy Template From")); + universalCopyItem.MenuItems.Add(new MenuItem("Copy Schedules")); + }; + + ContextMenu contextMenu = new(new[] { universalCopyItem }); + control.ContextMenu = contextMenu; + + IntPtr controlHandle = control.Handle; + IntPtr universalCopyHandle = universalCopyItem.Handle; + + Assert.NotEqual(IntPtr.Zero, controlHandle); + Assert.NotEqual(IntPtr.Zero, universalCopyHandle); + + // Act: simulate Windows delivering WM_INITMENUPOPUP when Universal Copy is hovered. + // Before the WM_INITMENUPOPUP fix in Control.WndProc, ProcessInitMenuPopup was never + // reached, Popup never fired, and the "" placeholder was never replaced. + SendMessage(controlHandle, WM_INITMENUPOPUP, universalCopyHandle, IntPtr.Zero); + + // Assert: exact submenu items visible in the screenshot. + Assert.Equal(4, universalCopyItem.MenuItems.Count); + Assert.Equal("New Copy Template", universalCopyItem.MenuItems[0].Text); + Assert.Equal("Edit Copy Template", universalCopyItem.MenuItems[1].Text); + Assert.Equal("New Copy Template From", universalCopyItem.MenuItems[2].Text); + Assert.Equal("Copy Schedules", universalCopyItem.MenuItems[3].Text); + } + + /// + /// Regression guard: sending WM_MENUSELECT a second time must not duplicate entries. + /// The Grid Colors handler clears and rebuilds on every Select; verifies idempotent + /// population when the user repeatedly moves the mouse away and back. + /// + [StaFact] + public void GridColors_SelectHandler_ReplacesItemsOnRepeatHover_NoDuplicates() + { + using Control control = new() { Visible = true }; + + MenuItem gridColorsItem = new("Grid Colors"); + gridColorsItem.MenuItems.Add(new MenuItem("Select Color Scheme")); + + int populateCount = 0; + gridColorsItem.Select += (_, _) => + { + populateCount++; + gridColorsItem.MenuItems.Clear(); + gridColorsItem.MenuItems.Add(new MenuItem($"Scheme A (call {populateCount})")); + gridColorsItem.MenuItems.Add(new MenuItem($"Scheme B (call {populateCount})")); + }; + + ContextMenu contextMenu = new(new[] { gridColorsItem }); + control.ContextMenu = contextMenu; + + IntPtr controlHandle = control.Handle; + IntPtr contextMenuHandle = contextMenu.Handle; + IntPtr wParam = unchecked((IntPtr)(0 | (MF_POPUP << 16))); + + // First hover + SendMessage(controlHandle, WM_MENUSELECT, wParam, contextMenuHandle); + Assert.Equal(2, gridColorsItem.MenuItems.Count); + Assert.Equal("Scheme A (call 1)", gridColorsItem.MenuItems[0].Text); + + // Second hover — must replace, not append + SendMessage(controlHandle, WM_MENUSELECT, wParam, contextMenuHandle); + Assert.Equal(2, gridColorsItem.MenuItems.Count); + Assert.Equal("Scheme A (call 2)", gridColorsItem.MenuItems[0].Text); + } +} From 268aaa43c4383c36c0a1214916596d20d1fe385f Mon Sep 17 00:00:00 2001 From: RX Date: Tue, 2 Jun 2026 21:13:04 +0800 Subject: [PATCH 6/6] Fix WM_MENUSELECT popup lookup to use native Win32 APIs The previous ProcessMenuSelect used managed MenuItems[index] to find the popup item, but hidden items (Visible = false) are skipped in the native menu. This caused a native-to-managed index mismatch: Windows reports the native index (which excludes hidden items), but the managed collection includes hidden items at earlier indices, so the wrong MenuItem was found. CargoWise's ZGrid context menu uses hidden items extensively (DeactivateMenuItem, ActivateMenuItem, CustomiseMenuItem, CopyToNewRow, etc.), which made every popup lookup return the wrong item. The fix replaces the index-based lookup with GetMenuItemFromNativeIndex(), which uses Win32 GetMenuItemID/GetSubMenu to navigate the actual native menu structure. This matches the approach used by the .NET Framework reference implementation (WinFormsLegacyControls) and is immune to index mismatches from hidden items. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ContextMenu/LegacyMenuInteropCompat.cs | 6 +++ .../Controls/Unsupported/ContextMenu/Menu.cs | 48 +++++++++++++++-- .../LazyMenuPopulationRegressionTests.cs | 51 +++++++++++++++++-- 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LegacyMenuInteropCompat.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LegacyMenuInteropCompat.cs index cc5e9fb3d3e..9903a3f8943 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LegacyMenuInteropCompat.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LegacyMenuInteropCompat.cs @@ -112,6 +112,12 @@ internal static class LegacyMenuUnsafeNativeMethods [DllImport(Libraries.User32, ExactSpelling = true)] public static extern bool SetMenuDefaultItem(HandleRef hMenu, int uItem, bool fByPos); + [DllImport(Libraries.User32, ExactSpelling = true)] + public static extern int GetMenuItemID(IntPtr hMenu, int nPos); + + [DllImport(Libraries.User32, ExactSpelling = true)] + public static extern IntPtr GetSubMenu(IntPtr hMenu, int nPos); + public static bool GetMenuItemInfo(HandleRef hMenu, int uItem, bool fByPosition, LegacyMenuNativeMethods.MENUITEMINFO_T lpmii) { bool result = GetMenuItemInfo(hMenu.Handle, uItem, fByPosition, lpmii); diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs index 00027796294..be078620bfa 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs @@ -645,11 +645,11 @@ internal bool ProcessMenuSelect(ref Message m) } else { - Menu menu = (m.LParamInternal == handle) ? this : FindMenuItem(FindHandle, m.LParamInternal); - if (menu is not null && item >= 0 && item < menu.ItemCount) - { - menuItem = menu.MenuItems[item]; - } + // Use native Win32 APIs to find the correct MenuItem. We cannot use + // managed MenuItems[index] because hidden items (Visible = false) are + // skipped in the native menu, causing the native index to differ from + // the managed collection index. + menuItem = GetMenuItemFromNativeIndex((IntPtr)(nint)m.LParamInternal, item); } } @@ -662,6 +662,44 @@ internal bool ProcessMenuSelect(ref Message m) return false; } + /// + /// Resolves a popup menu item from its native menu position using Win32 APIs. + /// For a popup item (one with a submenu), this retrieves the submenu handle via + /// GetSubMenu, then recursively searches the submenu's children to find + /// a managed whose is the + /// popup item we need. This approach is immune to index mismatches caused by + /// hidden (Visible = false) menu items. + /// + private static MenuItem? GetMenuItemFromNativeIndex(IntPtr hmenu, int index) + { + int id = UnsafeNativeMethods.GetMenuItemID(hmenu, index); + if (id == unchecked((int)0xFFFFFFFF)) + { + // The item is a popup — get its submenu handle and search children. + IntPtr childMenu = UnsafeNativeMethods.GetSubMenu(hmenu, index); + int count = UnsafeNativeMethods.GetMenuItemCount(new HandleRef(null, childMenu)); + for (int i = 0; i < count; i++) + { + MenuItem? child = GetMenuItemFromNativeIndex(childMenu, i); + if (child?.Parent is MenuItem parentItem) + { + return parentItem; + } + } + } + else + { + // Non-popup item — look up by command ID. + Command? command = Command.GetCommandFromID(id); + if (command?.Target is MenuItem.MenuItemData data) + { + return data.baseItem; + } + } + + return null; + } + protected internal virtual bool ProcessCmdKey(ref Message msg, Keys keyData) { MenuItem item = FindMenuItem(FindShortcut, (IntPtr)(int)keyData); diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs index 72aeec124d8..08fbe19e988 100644 --- a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/LazyMenuPopulationRegressionTests.cs @@ -79,12 +79,10 @@ public void GridColors_SelectHandler_PopulatesSubMenuItems_WhenWmMenuSelectRecei Assert.NotEqual(IntPtr.Zero, controlHandle); Assert.NotEqual(IntPtr.Zero, contextMenuHandle); - // "Grid Colors" is at index 0 in the context menu; it has children so Windows sets MF_POPUP. + // "Grid Colors" is at native index 0 in the context menu; it has children so Windows sets MF_POPUP. IntPtr wParam = unchecked((IntPtr)(0 | (MF_POPUP << 16))); // Act: simulate Windows delivering WM_MENUSELECT when the user hovers "Grid Colors". - // Before the fix, Control.WndProc fell through to DefWndProc; ProcessMenuSelect was - // never reached, MenuItem.Select never fired, and the placeholder items were not replaced. SendMessage(controlHandle, WM_MENUSELECT, wParam, contextMenuHandle); // Assert: the submenu must contain the live entries, not the original placeholders. @@ -94,6 +92,53 @@ public void GridColors_SelectHandler_PopulatesSubMenuItems_WhenWmMenuSelectRecei Assert.Equal("High Contrast", gridColorsItem.MenuItems[2].Text); } + /// + /// Regression test for the hidden-items index mismatch: when context menu items have + /// Visible = false, the native menu skips them, so the native index differs from + /// the managed index. The original ProcessMenuSelect used + /// MenuItems[nativeIndex] which returned the wrong item. The fix uses native + /// Win32 APIs (GetMenuItemID, GetSubMenu) to navigate the actual menu structure. + /// + [StaFact] + public void GridColors_SelectHandler_WorksWithHiddenItems_WhenNativeIndexDiffersFromManaged() + { + using Control control = new() { Visible = true }; + + // Build a context menu with hidden items before Grid Colors to create index mismatch. + MenuItem viewItem = new("View"); + MenuItem deactivateItem = new("Deactivate") { Visible = false }; + MenuItem activateItem = new("Activate") { Visible = false }; + MenuItem gridColorsItem = new("Grid Colors"); + gridColorsItem.MenuItems.Add(new MenuItem("Placeholder")); + + bool selectFired = false; + gridColorsItem.Select += (_, _) => + { + selectFired = true; + gridColorsItem.MenuItems.Clear(); + gridColorsItem.MenuItems.Add(new MenuItem("Scheme A")); + gridColorsItem.MenuItems.Add(new MenuItem("Scheme B")); + }; + + // Managed indices: View=0, Deactivate=1(hidden), Activate=2(hidden), GridColors=3 + // Native indices: View=0, GridColors=1 (hidden items skipped) + ContextMenu contextMenu = new(new[] { viewItem, deactivateItem, activateItem, gridColorsItem }); + control.ContextMenu = contextMenu; + + IntPtr controlHandle = control.Handle; + IntPtr contextMenuHandle = contextMenu.Handle; + + // Grid Colors is at NATIVE index 1 (after View), not managed index 3. + IntPtr wParam = unchecked((IntPtr)(1 | (MF_POPUP << 16))); + + SendMessage(controlHandle, WM_MENUSELECT, wParam, contextMenuHandle); + + Assert.True(selectFired, "Select event should fire on Grid Colors despite hidden items shifting the native index."); + Assert.Equal(2, gridColorsItem.MenuItems.Count); + Assert.Equal("Scheme A", gridColorsItem.MenuItems[0].Text); + Assert.Equal("Scheme B", gridColorsItem.MenuItems[1].Text); + } + /// /// Universal Copy pattern (ZFilterGridModule.AddUniversalCopyMenuItems via /// ): the "Universal Copy" submenu starts with a single