Too soon, it is still not finished.

The problem was with:

m_menu->setVisible(m_visible);

setters/getters should not have side effects or any logic, but this method has. Detached menu stayed as top level window and rendered itself.

VLC has a bug, each systray menu update creates new QMenu, but old one is not deleted:
https://github.com/videolan/vlc/blob/master/modules/gui/qt/menus.cpp#L655

QMenu::clear() deletes all attached actions (if not used somewhere else) but not menus! It is weird, but someone explained it here:
https://bugreports.qt.io/browse/QTBUG-11070

tested successfully locally. I don't see menus opening on the top left corner when running vlc or hp-systray.

I will send fixes in 5 minutes

Off-topic idea: This QPA integration uses KStatusNotifierItem, which then translates it to DBus. Wouldn't it be better to talk to DBus directly? From the other side, this may be a duplication of work... Was the idea discussed?

In D25223#561561, @kmaterka wrote:

Off-topic idea: This QPA integration uses KStatusNotifierItem, which then translates it to DBus. Wouldn't it be better to talk to DBus directly? From the other side, this may be a duplication of work... Was the idea discussed?

We did discuss it.

IMHO the best solution would be to reuse the DBus tray implementation that is part of Qt (https://code.woboq.org/qt5/qtbase/src/platformsupport/themes/genericunix/dbustray/qdbustrayicon_p.h.html). However, it is part of the genericunix theme and thus not "public". @davidedmundson had an idea on how we can "inherit" from the gericunix theme and reuse the implementation

I had this. I abandoned because we ended up forking some special wayland stuff in our DBus menu, so would always want our implementation.

?https://codereview.qt-project.org/c/qt/qtbase/+/168458

In D25223#561570, @davidedmundson wrote:

I had this. I abandoned because we ended up forking some special wayland stuff in our DBus menu, so would always want our implementation.

?https://codereview.qt-project.org/c/qt/qtbase/+/168458

Having an option to fallback to default/predefined theme would be great, but I guess KDE needs to have it's own SystemTray implementation anyway (?). Some cleanup is required, because currently:

• in Plasma-integration (QPA theme):
  • there is a forked QDBusMenuBar from Qt
  • for tray icon it uses KStatusNotifierItem which has it's drawbacks (additional layer)
• in KNotifications, KStatusNotifierItem uses libdbusmenu-qt (not forked). It is optional dependency, is there any reason for that in 2019?
• in plasma-workspace StatusNotifierItemSource uses DBusMenuImporter forked from libdbusmenu-qt
• Qt has it's own implementation of DBus menu exporter:
  • in genericunix qpa theme, unfortuantelly QDBusPlatformMenu is private
  • it uses undocumented "NewMenu" signal
  • and is buggy: QTBUG-79287, KDE 383202
• libdbusmenu-qt was not actively updated/maintained for years, I see that David made few commits in 2015. Shouldn't DBusMenuImporter be upstreamed?
• Freedesktop spec is still in draft phase

Anyway, this is not part of this code review, sorry for the off topic.

My long-term goal is to get rid of the application side KStatusNotifierItem and amend the QSystemTrayIcon API

In D25223#561638, @nicolasfella wrote:

My long-term goal is to get rid of the application side KStatusNotifierItem and amend the QSystemTrayIcon API

OK, I will think about this. This change is only to fix one regression.
@broulik Is it OK now? Or should I use std::optional (and require C++17)?