Skip to content

[Windows] MenuItem properties set before addItem() are silently ignored (state, enabled) #5

@lijy91

Description

@lijy91

Description

On Windows, MenuItem.state and MenuItem.enabled properties are silently ignored when set before the item is added to a Menu via addItem() or insertItem(). This applies to all platforms when using the library, but the root cause is in the Windows native implementation.

Steps to Reproduce

Run the following code on Windows:

final menu = Menu();

final checkableItem = MenuItem("Checkable", MenuItemType.checkbox);
checkableItem.state = MenuItemState.checked;   // ← Set before addItem

final disabledItem = MenuItem("Disabled");
disabledItem.enabled = false;                  // ← Set before addItem

menu.addItem(checkableItem);
menu.addItem(disabledItem);

menu.open(PositioningStrategy.cursorPosition());

Expected: The "Checkable" item shows a checkmark, and the "Disabled" item is grayed out.

Actual: The "Checkable" item has no checkmark, and the "Disabled" item appears enabled.

Root Cause

The Windows native implementation in menu_windows.cpp guards SetEnabled and SetState with a check on pimpl_->parent_menu_:

MenuItem::SetEnabled() (line 357):

void MenuItem::SetEnabled(bool enabled) {
  if (pimpl_->parent_menu_) {    // ← null if called before addItem
    EnableMenuItem(pimpl_->parent_menu_, pimpl_->id_, enabled ? MF_ENABLED : MF_GRAYED);
  }
}

MenuItem::SetState() (line 373):

void MenuItem::SetState(MenuItemState state) {
  if (pimpl_->type_ == MenuItemType::Checkbox || ...) {
    pimpl_->state_ = state;
    if (pimpl_->parent_menu_) {  // ← null if called before addItem
      CheckMenuItem(pimpl_->parent_menu_, pimpl_->id_, check_state);
    }
  }
}

The parent_menu_ field is only assigned in Menu::AddItem() after AppendMenuW():

void Menu::AddItem(std::shared_ptr<MenuItem> item) {
  ...
  AppendMenuW(pimpl_->hmenu_, flags, menu_id, w_label_str.c_str());

  // Update the item's impl with menu info
  item->pimpl_->parent_menu_ = pimpl_->hmenu_;
}

Additionally, AddItem() hardcodes MF_UNCHECKED for checkbox/radio types instead of reading the item's current state_, so the default flags always win:

} else if (item->GetType() == MenuItemType::Checkbox) {
    flags |= MF_UNCHECKED;  // Always unchecked, ignores pre-set state
}

Workaround

Set state and enabled after calling addItem():

final menu = Menu();

final checkableItem = MenuItem("Checkable", MenuItemType.checkbox);
final disabledItem = MenuItem("Disabled");

menu.addItem(checkableItem);
menu.addItem(disabledItem);

// Set AFTER addItem
checkableItem.state = MenuItemState.checked;
disabledItem.enabled = false;

Platform Comparison

Platform Works when set before addItem?
macOS ✅ Yes (NSMenuItem operates independently)
Linux ✅ Yes (GTK widget created in constructor)
Windows ❌ No (parent_menu_ is null until addItem)

Windows is the only platform with this limitation.

Suggested Fix

In Menu::AddItem() and Menu::InsertItem(), re-apply the item's current state and enabled status after setting parent_menu_:

// After setting parent_menu_
item->pimpl_->parent_menu_ = pimpl_->hmenu_;

// Re-apply properties that were set before addItem
item->SetState(item->GetState());
item->SetEnabled(item->IsEnabled());

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions