-
Notifications
You must be signed in to change notification settings - Fork 0
Restore legacy menu message parity #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
3d70e95
4460f20
cdf9a16
954acdb
7a1eb9c
268aaa4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
| } | ||
|
|
@@ -11278,6 +11272,38 @@ internal void WmContextMenu(ref Message m, Control sourceControl) | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles the WM_MENUSELECT message. | ||
| /// </summary> | ||
| 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 | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles the WM_EXITMENULOOP message. If this control has a context menu, its | ||
| /// Collapse event is raised. | ||
| /// </summary> | ||
| private void WmExitMenuLoop(ref Message m) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| { | ||
| #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 | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles the WM_INITMENUPOPUP message. Dispatches to the legacy <see cref="ContextMenu"/> so | ||
| /// that <see cref="MenuItem.Popup"/> events fire for submenus. Without this, controls that own | ||
|
|
@@ -12765,8 +12791,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. | ||
|
|
@@ -13078,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); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| [DllImport(Libraries.User32, ExactSpelling = true)] | ||
| public static extern IntPtr GetSubMenu(IntPtr hMenu, int nPos); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| public static bool GetMenuItemInfo(HandleRef hMenu, int uItem, bool fByPosition, LegacyMenuNativeMethods.MENUITEMINFO_T lpmii) | ||
| { | ||
| bool result = GetMenuItemInfo(hMenu.Handle, uItem, fByPosition, lpmii); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -624,6 +624,82 @@ 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 | ||
| { | ||
| // 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); | ||
| } | ||
| } | ||
|
|
||
| if (menuItem is not null) | ||
| { | ||
| menuItem.PerformSelect(); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 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 | ||
| /// <c>GetSubMenu</c>, then recursively searches the submenu's children to find | ||
| /// a managed <see cref="MenuItem"/> whose <see cref="MenuItem.Parent"/> is the | ||
| /// 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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| { | ||
| 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); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7004,6 +7004,21 @@ private void WmInitMenuPopup(ref Message m) | |
| base.WndProc(ref m); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles the WM_MENUSELECT message. | ||
| /// </summary> | ||
| private void WmMenuSelect(ref Message m) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| { | ||
| #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); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles the WM_MENUCHAR message. | ||
| /// </summary> | ||
|
|
@@ -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; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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