kmaterka added a comment.
In D24755#550415 <https://phabricator.kde.org/D24755#550415>, @anthonyfieroni wrote: > That should be fine, in QPA we have a tray icon per sni, update menu should be on same object which will not be a problem, check it. There are two objects in QPA that live independently: - KDEPlatformSystemTrayIcon (QPlatformSystemTrayIcon), with KSNI instance, KSNI and KDEPlatformSystemTrayIcon **are** destroyed on QSystemTrayIcon->hide() and new instance (with new KSNI) is created on QSystemTrayIcon->show() - SystemTrayMenu (QPlatformMenu) is **not** destroyed on QSystemTrayIcon->hide() and will be reused later on QSystemTrayIcon->show() kdeplatformsystemtrayicon.cpp#L339 <https://github.com/KDE/plasma-integration/blob/master/src/platformtheme/kdeplatformsystemtrayicon.cpp#L339> void KDEPlatformSystemTrayIcon::updateMenu(QPlatformMenu *menu) { //... if (SystemTrayMenu *ourMenu = qobject_cast<SystemTrayMenu*>(menu)) { m_sni->setContextMenu(ourMenu->menu()); } } About you patch: I understand your idea, but it changes API contract and is not backward-compatible. Current documentation says: > The KStatusNotifierItem instance takes ownership of the menu, and will delete it upon its destruction. This is quite clear, I want to be really careful here - I don't want to be blamed for memory leaks :) I think that we need to keep: > d->menu->setParent(nullptr); in setContextMenu. Then, to prevent menu deletion, developer needs to explicitly retake ownership, for example: QMenu *menu = ourMenu->menu() QWidget *parent = menu->parent(); m_sni->setContextMenu(menu); menu->setParent(parent); The problem is that SystemTrayMenu->menu() has no parent and there is no QWidget to use... REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D24755 To: kmaterka, #frameworks, davidedmundson, broulik, nicolasfella Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
