Restore legacy menu message parity by Rick-Xu-WTG · Pull Request #21 · WiseTechGlobal/winforms

Rick-Xu-WTG · 2026-06-01T10:52:19Z

This pull request improves the handling of legacy context menu messages in Windows Forms controls, ensuring correct event dispatch and fixing issues with menu item selection and popup events. The changes also include test updates and some code cleanup.

Context Menu Message Handling Improvements:

Added explicit handling for WM_MENUSELECT and WM_EXITMENULOOP messages in Control and Form classes, ensuring that legacy context menu events (like MenuItem.Popup and selection) are properly dispatched. [1] [2] [3] [4]
Implemented ProcessMenuSelect logic in Menu to reliably resolve the correct MenuItem for selection, including complex cases with hidden items, by using Win32 interop (GetMenuItemID, GetSubMenu). [1] [2]

Code Simplification:

Replaced verbose null checks with null-conditional operators for attaching/detaching Disposed event handlers in ContextMenu and ContextMenuStrip properties. [1] [2]

Testing and Documentation:

Moved and updated the ContextMenuSubMenuPopupTests to a new location, clarifying test comments and improving assertion style. [1] [2] [3] [4]## Summary

Restore WM_MENUSELECT routing in the new WinForms tree so legacy MenuItem.Select behavior is preserved for both command and popup items.
Restore WM_INITMENUPOPUP routing so submenu Popup handlers fire and dynamic menu items can be populated.
Add LazyMenuPopulationRegressionTests proving the full end-to-end message path for both the Grid Colors (Select-based) and Universal Copy (Popup-based) lazy-loading patterns.
Keep the regression coverage in the new unit test tree under src/test/unit/System.Windows.Forms/System/Windows/Forms/Controls/Unsupported/ContextMenu/.

Rationale

The legacy CargoWise WinForms repo already routed these menu messages correctly; the new tree needed parity, not an app-level workaround.
App-layer workarounds (Select→Popup) in GridColourSchemeManager and UniversalCopyManager are not needed once this framework fix is in place.

Validation

dotnet build src\test\unit\System.Windows.Forms\System.Windows.Forms.Tests.csproj --no-restore ✅
LazyMenuPopulationRegressionTests committed and covers: Grid Colors WM_MENUSELECT path, Universal Copy WM_INITMENUPOPUP path, idempotency (no duplicate items on repeat hover).
Local NuGet cache updated and verified: ProcessMenuSelect in Menu, WmMenuSelect in Control and Form.
Review notes saved on the desktop: C:\Users\Rick.Xu\OneDrive - WiseTech Global Pty Ltd\Desktop\wm-menu-select-root-fix-report.md.

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>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…py 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>

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>

FahmiFuzi · 2026-06-04T05:31:58Z

    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);


The legacy has implementation for this not just the contract. Why you did not have this one here?https://github.com/WiseTechGlobal/winforms/blob/net8.0_legacy/src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.GetMenuItemID.cs#L15-L20

FahmiFuzi · 2026-06-04T05:33:22Z

+    public static extern int GetMenuItemID(IntPtr hMenu, int nPos);
+
+    [DllImport(Libraries.User32, ExactSpelling = true)]
+    public static extern IntPtr GetSubMenu(IntPtr hMenu, int nPos);


Same goes to the GetSubMenu, the implementation is there too: https://github.com/WiseTechGlobal/winforms/blob/net8.0_legacy/src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.GetSubMenu.cs#L15-L20

FahmiFuzi · 2026-06-04T05:35:57Z

+        ///  popup item we need.  This approach is immune to index mismatches caused by
+        ///  hidden (<c>Visible = false</c>) menu items.
+        /// </summary>
+        private static MenuItem? GetMenuItemFromNativeIndex(IntPtr hmenu, int index)


Can you point me or add link to where this implementation came from in the legacycode? It would be easier to reference later, so far i cant find it anywhere.

FahmiFuzi · 2026-06-04T05:40:22Z

+    /// <summary>
+    ///  Handles the WM_MENUSELECT message.
+    /// </summary>
+    private void WmMenuSelect(ref Message m)


Any reasons why the implementation differ from the original one, rather than doing lift and shift?
https://github.com/WiseTechGlobal/winforms/blob/net8.0_legacy/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L221-L255

FahmiFuzi · 2026-06-04T05:41:47Z

+    ///  Handles the WM_EXITMENULOOP message. If this control has a context menu, its
+    ///  Collapse event is raised.
+    /// </summary>
+    private void WmExitMenuLoop(ref Message m)


Similarly here too, why change from the original one? https://github.com/WiseTechGlobal/winforms/blob/net8.0_legacy/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L12230-L12244

FahmiFuzi · 2026-06-04T05:43:18Z

+    /// <summary>
+    ///  Handles the WM_MENUSELECT message.
+    /// </summary>
+    private void WmMenuSelect(ref Message m)


Alot different than original too here: https://github.com/WiseTechGlobal/winforms/blob/net8.0_legacy/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L221-L255

FahmiFuzi · 2026-06-04T05:44:33Z

+///   <c>ZFilterGridModule.AddUniversalCopyMenuItems</c> (Popup-based).
+///  </para>
+/// </remarks>
+public class LazyMenuPopulationRegressionTests


Where this test is coming from? Are you adding a new one? if yes, does the existing from legacy does not cover for this?

FahmiFuzi · 2026-06-04T05:45:19Z

+
+namespace System.Windows.Forms.Tests;
+
+public class MenuSelectTests


Similarly here, is this a new test? I cant find from the legacy branch,

Fix legacy menu message parity

3d70e95

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>

Rick-Xu-WTG force-pushed the squad/wm-menu-select-root-fix branch from b855d59 to 3d70e95 Compare June 1, 2026 10:53

Rick-Xu-WTG and others added 5 commits June 1, 2026 18:57

Fix WM_MENUSELECT regression tests

4460f20

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove in-repo report

cdf9a16

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Simplify null check

954acdb

srijan-dev8 approved these changes Jun 3, 2026

View reviewed changes

FahmiFuzi reviewed Jun 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore legacy menu message parity#21

Restore legacy menu message parity#21
Rick-Xu-WTG wants to merge 6 commits into
masterfrom
squad/wm-menu-select-root-fix

Rick-Xu-WTG commented Jun 1, 2026 •

edited

Loading

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

FahmiFuzi Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		namespace System.Windows.Forms.Tests;

		public class MenuSelectTests

Conversation

Rick-Xu-WTG commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rick-Xu-WTG commented Jun 1, 2026 •

edited

Loading