Skip to content

Commit 43139e2

Browse files
lijy91lijy91
committed
Add submenu event listener management to MenuItem
Introduces listener IDs to MenuItem for submenu opened/closed events, ensuring listeners are properly added and removed when submenus are set or removed. Enhances resource management and prevents potential event leaks by cleaning up listeners in destructor and relevant methods.
1 parent 89e2b1b commit 43139e2

File tree

3 files changed

+96
-54
lines changed

3 files changed

+96
-54
lines changed

‎src/platform/macos/menu_macos.mm‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -271,11 +271,7 @@ - (void)menuDidClose:(NSMenu*)menu {
271271

272272
// 设置默认的 Block 处理器，直接发送事件
273273
pimpl_->ns_menu_item_target_.clickedBlock = ^(MenuItemId item_id) {
274-
try {
275-
Emit<MenuItemClickedEvent>(item_id);
276-
} catch (...) {
277-
// Protect against event emission exceptions
278-
}
274+
Emit<MenuItemClickedEvent>(item_id);
279275
};
280276
}
281277

@@ -288,11 +284,7 @@ - (void)menuDidClose:(NSMenu*)menu {
288284

289285
// 设置默认的 Block 处理器，直接发送事件
290286
pimpl_->ns_menu_item_target_.clickedBlock = ^(MenuItemId item_id) {
291-
try {
292-
Emit<MenuItemClickedEvent>(item_id);
293-
} catch (...) {
294-
// Protect against event emission exceptions
295-
}
287+
Emit<MenuItemClickedEvent>(item_id);
296288
};
297289
}
298290

@@ -471,20 +463,12 @@ - (void)menuDidClose:(NSMenu*)menu {
471463
MenuItem* self = this;
472464
pimpl_->submenu_opened_listener_id_ = submenu->AddListener<MenuOpenedEvent>(
473465
[self, menu_item_id](const MenuOpenedEvent& event) {
474-
try {
475-
self->Emit<MenuItemSubmenuOpenedEvent>(menu_item_id);
476-
} catch (...) {
477-
// Protect against event emission exceptions
478-
}
466+
self->Emit<MenuItemSubmenuOpenedEvent>(menu_item_id);
479467
});
480468

481469
pimpl_->submenu_closed_listener_id_ = submenu->AddListener<MenuClosedEvent>(
482470
[self, menu_item_id](const MenuClosedEvent& event) {
483-
try {
484-
self->Emit<MenuItemSubmenuClosedEvent>(menu_item_id);
485-
} catch (...) {
486-
// Protect against event emission exceptions
487-
}
471+
self->Emit<MenuItemSubmenuClosedEvent>(menu_item_id);
488472
});
489473
}
490474
} else {
@@ -581,19 +565,11 @@ - (void)menuDidClose:(NSMenu*)menu {
581565

582566
// 设置默认的 Block 处理器，直接发送事件
583567
pimpl_->delegate_.openedBlock = ^(MenuId menu_id) {
584-
try {
585-
Emit<MenuOpenedEvent>(menu_id);
586-
} catch (...) {
587-
// Protect against event emission exceptions
588-
}
568+
Emit<MenuOpenedEvent>(menu_id);
589569
};
590570

591571
pimpl_->delegate_.closedBlock = ^(MenuId menu_id) {
592-
try {
593-
Emit<MenuClosedEvent>(menu_id);
594-
} catch (...) {
595-
// Protect against event emission exceptions
596-
}
572+
Emit<MenuClosedEvent>(menu_id);
597573
};
598574
}
599575

‎src/platform/macos/tray_icon_macos.mm‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
static const void* kTrayIconIdKey = &kTrayIconIdKey;
2222

2323
@interface NSStatusBarButtonTarget : NSObject
24-
@property(nonatomic, copy) TrayIconClickedBlock left_clicked_callback;
25-
@property(nonatomic, copy) TrayIconRightClickedBlock right_clicked_callback;
26-
@property(nonatomic, copy) TrayIconDoubleClickedBlock double_clicked_callback;
24+
@property(nonatomic, copy) TrayIconClickedBlock left_clicked_callback_;
25+
@property(nonatomic, copy) TrayIconRightClickedBlock right_clicked_callback_;
26+
@property(nonatomic, copy) TrayIconDoubleClickedBlock double_clicked_callback_;
2727
- (void)handleStatusItemEvent:(id)sender;
2828
@end
2929

@@ -114,9 +114,9 @@ void CleanupEventHandlers() {
114114

115115
// Clean up blocks first
116116
if (ns_status_bar_button_target_) {
117-
ns_status_bar_button_target_.left_clicked_callback = nil;
118-
ns_status_bar_button_target_.right_clicked_callback = nil;
119-
ns_status_bar_button_target_.double_clicked_callback = nil;
117+
ns_status_bar_button_target_.left_clicked_callback_ = nil;
118+
ns_status_bar_button_target_.right_clicked_callback_ = nil;
119+
ns_status_bar_button_target_.double_clicked_callback_ = nil;
120120
ns_status_bar_button_target_ = nil;
121121
}
122122

@@ -167,15 +167,15 @@ void CleanupEventHandlers() {
167167

168168
// Set up click handler blocks
169169
if (pimpl_->ns_status_bar_button_target_) {
170-
pimpl_->ns_status_bar_button_target_.left_clicked_callback = ^{
170+
pimpl_->ns_status_bar_button_target_.left_clicked_callback_ = ^{
171171
Emit<TrayIconClickedEvent>(pimpl_->id_);
172172
};
173173

174-
pimpl_->ns_status_bar_button_target_.right_clicked_callback = ^{
174+
pimpl_->ns_status_bar_button_target_.right_clicked_callback_ = ^{
175175
Emit<TrayIconRightClickedEvent>(pimpl_->id_);
176176
};
177177

178-
pimpl_->ns_status_bar_button_target_.double_clicked_callback = ^{
178+
pimpl_->ns_status_bar_button_target_.double_clicked_callback_ = ^{
179179
Emit<TrayIconDoubleClickedEvent>(pimpl_->id_);
180180
};
181181
}
@@ -393,18 +393,18 @@ - (void)handleStatusItemEvent:(id)sender {
393393
(event.type == NSEventTypeLeftMouseUp &&
394394
(event.modifierFlags & NSEventModifierFlagControl))) {
395395
// Right click or Ctrl+Left click
396-
if (_right_clicked_callback) {
397-
_right_clicked_callback();
396+
if (_right_clicked_callback_) {
397+
_right_clicked_callback_();
398398
}
399399
} else if (event.type == NSEventTypeLeftMouseUp) {
400400
// Check for double click
401401
if (event.clickCount == 2) {
402-
if (_double_clicked_callback) {
403-
_double_clicked_callback();
402+
if (_double_clicked_callback_) {
403+
_double_clicked_callback_();
404404
}
405405
} else {
406-
if (_left_clicked_callback) {
407-
_left_clicked_callback();
406+
if (_left_clicked_callback_) {
407+
_left_clicked_callback_();
408408
}
409409
}
410410
}

‎src/platform/windows/menu_windows.cpp‎

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ class MenuItem::Impl {
103103
std::shared_ptr<Menu> submenu_;
104104
int window_proc_handle_id_;
105105
std::function<void(MenuItemId)> clicked_callback_;
106+
size_t submenu_opened_listener_id_;
107+
size_t submenu_closed_listener_id_;
106108

107109
Impl(MenuItemId id, HMENU parent_menu, MenuItemType type)
108110
: id_(id),
@@ -112,13 +114,26 @@ class MenuItem::Impl {
112114
has_accelerator_(false),
113115
state_(MenuItemState::Unchecked),
114116
radio_group_(-1),
115-
window_proc_handle_id_(-1) {}
117+
window_proc_handle_id_(-1),
118+
submenu_opened_listener_id_(0),
119+
submenu_closed_listener_id_(0) {}
116120

117121
~Impl() {
118122
// Unregister window procedure handler
119123
if (window_proc_handle_id_ != -1) {
120124
WindowMessageDispatcher::GetInstance().UnregisterHandler(window_proc_handle_id_);
121125
}
126+
127+
// Remove submenu listeners before cleaning up submenu reference
128+
if (submenu_ && submenu_opened_listener_id_ != 0) {
129+
submenu_->RemoveListener(submenu_opened_listener_id_);
130+
submenu_opened_listener_id_ = 0;
131+
}
132+
if (submenu_ && submenu_closed_listener_id_ != 0) {
133+
submenu_->RemoveListener(submenu_closed_listener_id_);
134+
submenu_closed_listener_id_ = 0;
135+
}
136+
122137
// Windows menu items are automatically cleaned up when the menu is
123138
// destroyed
124139
}
@@ -307,21 +322,58 @@ int MenuItem::GetRadioGroup() const {
307322
}
308323

309324
void MenuItem::SetSubmenu(std::shared_ptr<Menu> submenu) {
325+
// Remove previous submenu listeners if they exist
326+
if (pimpl_->submenu_ && pimpl_->submenu_opened_listener_id_ != 0) {
327+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_opened_listener_id_);
328+
pimpl_->submenu_opened_listener_id_ = 0;
329+
}
330+
if (pimpl_->submenu_ && pimpl_->submenu_closed_listener_id_ != 0) {
331+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_closed_listener_id_);
332+
pimpl_->submenu_closed_listener_id_ = 0;
333+
}
334+
310335
pimpl_->submenu_ = submenu;
336+
337+
// Update platform menu if parent_menu_ is set
311338
if (pimpl_->parent_menu_ && submenu) {
312339
MENUITEMINFOW mii = {};
313340
mii.cbSize = sizeof(MENUITEMINFOW);
314341
mii.fMask = MIIM_SUBMENU;
315342
mii.hSubMenu = static_cast<HMENU>(submenu->GetNativeObject());
316343
SetMenuItemInfoW(pimpl_->parent_menu_, pimpl_->id_, FALSE, &mii);
317344
}
345+
346+
// Add event listeners to forward submenu events (independent of parent_menu_)
347+
if (submenu) {
348+
MenuItemId menu_item_id = pimpl_->id_;
349+
MenuItem* self = this;
350+
pimpl_->submenu_opened_listener_id_ = submenu->AddListener<MenuOpenedEvent>(
351+
[self, menu_item_id](const MenuOpenedEvent& event) {
352+
self->Emit<MenuItemSubmenuOpenedEvent>(menu_item_id);
353+
});
354+
355+
pimpl_->submenu_closed_listener_id_ = submenu->AddListener<MenuClosedEvent>(
356+
[self, menu_item_id](const MenuClosedEvent& event) {
357+
self->Emit<MenuItemSubmenuClosedEvent>(menu_item_id);
358+
});
359+
}
318360
}
319361

320362
std::shared_ptr<Menu> MenuItem::GetSubmenu() const {
321363
return pimpl_->submenu_;
322364
}
323365

324366
void MenuItem::RemoveSubmenu() {
367+
// Remove listeners before clearing submenu reference
368+
if (pimpl_->submenu_ && pimpl_->submenu_opened_listener_id_ != 0) {
369+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_opened_listener_id_);
370+
pimpl_->submenu_opened_listener_id_ = 0;
371+
}
372+
if (pimpl_->submenu_ && pimpl_->submenu_closed_listener_id_ != 0) {
373+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_closed_listener_id_);
374+
pimpl_->submenu_closed_listener_id_ = 0;
375+
}
376+
325377
pimpl_->submenu_.reset();
326378
if (pimpl_->parent_menu_) {
327379
MENUITEMINFOW mii = {};
@@ -375,10 +427,17 @@ class Menu::Impl {
375427
// This is our menu being opened
376428
// Emit menu opened event via callback
377429
if (opened_callback_) {
378-
try {
379-
opened_callback_(id_);
380-
} catch (...) {
381-
// Protect against event emission exceptions
430+
opened_callback_(id_);
431+
}
432+
} else {
433+
// Check if this is a submenu of one of our items
434+
for (const auto& item : items_) {
435+
auto submenu = item->GetSubmenu();
436+
if (submenu && submenu->GetNativeObject() == popup_menu) {
437+
// This is a submenu of one of our items being opened
438+
// The MenuItem will handle emitting MenuItemSubmenuOpenedEvent
439+
// through its event listeners
440+
break;
382441
}
383442
}
384443
}
@@ -390,10 +449,17 @@ class Menu::Impl {
390449
// This is our menu being closed
391450
// Emit menu closed event via callback
392451
if (closed_callback_) {
393-
try {
394-
closed_callback_(id_);
395-
} catch (...) {
396-
// Protect against event emission exceptions
452+
closed_callback_(id_);
453+
}
454+
} else {
455+
// Check if this is a submenu of one of our items
456+
for (const auto& item : items_) {
457+
auto submenu = item->GetSubmenu();
458+
if (submenu && submenu->GetNativeObject() == popup_menu) {
459+
// This is a submenu of one of our items being closed
460+
// The MenuItem will handle emitting MenuItemSubmenuClosedEvent
461+
// through its event listeners
462+
break;
397463
}
398464
}
399465
}

0 commit comments

Comments
 (0)