Skip to content

Commit 218de07

Browse files
committed
Fix Top End placement and submenu detach/reattach on Windows
WinUI 3: show the flyout against the 1x1 anchor element instead of a Position point. With a point the flyout ignored the edge alignment, so Top End opened left-aligned. Win32: MenuItem::SetSubmenu(nullptr) never reached the native menu, and an item appended with a submenu has no command ID of its own, so the lookup by ID missed it anyway. Find the item by position, and replace it with RemoveMenu plus InsertMenuItem: replacing hSubMenu in place destroys the old submenu, which the Menu object still owns and may attach again. Found by the menu GUI test (tools/gui/flutter_menu_test.ps1 in the workspace).
1 parent 373526f commit 218de07

2 files changed

Lines changed: 33 additions & 8 deletions

File tree

‎src/platform/windows/menu_windows.cpp‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -433,15 +433,39 @@ void MenuItem::SetSubmenu(std::shared_ptr<Menu> submenu) {
433433
pimpl_->submenu_closed_listener_id_ = 0;
434434
}
435435

436+
std::shared_ptr<Menu> old = pimpl_->submenu_;
436437
pimpl_->submenu_ = submenu;
437438

438-
// Update platform menu if parent_menu_ is set
439-
if (pimpl_->parent_menu_ && submenu) {
440-
MENUITEMINFOW mii = {};
441-
mii.cbSize = sizeof(MENUITEMINFOW);
442-
mii.fMask = MIIM_SUBMENU;
443-
mii.hSubMenu = static_cast<HMENU>(submenu->GetNativeObject());
444-
SetMenuItemInfoW(pimpl_->parent_menu_, pimpl_->id_, FALSE, &mii);
439+
// Update platform menu if parent_menu_ is set; a null submenu detaches the old one.
440+
if (pimpl_->parent_menu_) {
441+
// An item added with a submenu was appended with MF_POPUP and has no command ID of its
442+
// own, so a lookup by ID misses it: find it by position (its ID or its old submenu).
443+
HMENU old_submenu = old ? static_cast<HMENU>(old->GetNativeObject()) : nullptr;
444+
int count = GetMenuItemCount(pimpl_->parent_menu_);
445+
for (int i = 0; i < count; ++i) {
446+
MENUITEMINFOW info = {};
447+
info.cbSize = sizeof(MENUITEMINFOW);
448+
info.fMask = MIIM_ID | MIIM_SUBMENU;
449+
if (!GetMenuItemInfoW(pimpl_->parent_menu_, i, TRUE, &info)) continue;
450+
if (info.wID != pimpl_->id_ && !(old_submenu && info.hSubMenu == old_submenu)) continue;
451+
// Replacing hSubMenu through SetMenuItemInfo destroys the old submenu, which the Menu
452+
// object still owns (and may attach again). RemoveMenu keeps it alive: remove the item
453+
// and insert it again at the same position.
454+
RemoveMenu(pimpl_->parent_menu_, i, MF_BYPOSITION);
455+
std::wstring label = StringToWString(pimpl_->label_.value_or(""));
456+
MENUITEMINFOW mii = {};
457+
mii.cbSize = sizeof(MENUITEMINFOW);
458+
mii.fMask = MIIM_ID | MIIM_SUBMENU | MIIM_STRING | MIIM_STATE | MIIM_FTYPE | MIIM_BITMAP;
459+
mii.fType = MFT_STRING;
460+
mii.fState = (pimpl_->state_ == MenuItemState::Checked ? MFS_CHECKED : MFS_UNCHECKED) |
461+
(pimpl_->enabled_ ? MFS_ENABLED : MFS_GRAYED);
462+
mii.wID = static_cast<UINT>(pimpl_->id_);
463+
mii.hSubMenu = submenu ? static_cast<HMENU>(submenu->GetNativeObject()) : nullptr;
464+
mii.dwTypeData = const_cast<LPWSTR>(label.c_str());
465+
mii.hbmpItem = pimpl_->menu_bitmap_;
466+
InsertMenuItemW(pimpl_->parent_menu_, i, TRUE, &mii);
467+
break;
468+
}
445469
}
446470

447471
// Add event listeners to forward submenu events (independent of parent_menu_)

‎src/platform/windows/menu_winui3_windows.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,9 @@ bool WinUI3MenuSession::Open(Menu& menu, HWND owner, POINT anchor, Placement pla
235235
if (!impl.done) {
236236
impl.Build(menu, impl.flyout.Items());
237237
if (!impl.flyout.Items().Size()) winrt::throw_hresult(E_INVALIDARG);
238+
// Place against the 1x1 root at the anchor, not a Position point: with a point the
239+
// flyout ignores the edge alignment (TopEdgeAlignedRight opened left-aligned).
238240
P::FlyoutShowOptions options;
239-
options.Position(winrt::Windows::Foundation::Point{0, 0});
240241
options.Placement(ConvertPlacement(placement));
241242
// ShowAt before the island's first layout can silently fail to present.
242243
impl.loaded_event = impl.root.Loaded(winrt::auto_revoke, [&impl, options](auto&&, auto&&) {

0 commit comments

Comments
 (0)