Custom code for system tray icons can be dropped

[mitya57]

LXQt's platform theme currently has some custom code for handling QSystemTrayIcons and converting them into StatusNotifierItems based on libdbusmenu-qt.

However, Qt has a built-in implementation of the same since version 5.5 (however it became actually usable in 5.6.1). So I believe the custom code can be dropped in favour of code in Qt.

Moreover, it appears that the code in LXQt is buggy and can lead to crashes sometimes. A test case is available in QTBUG-54162 (which led me to creating this bug report).

[jleclanche]

cc @palinek

[paulolieuthier]

I'd rather we fix our implementation, as Qt's is private and thus we shouldn't rely on it.

[palinek]

How about using our implementation only in pre-5.6.1 versions ?

[palinek]

Have a look on the proposal lxqt/lxqt-qtplugin#11

[paulolieuthier]

I tested, and it makes @mitya57's test case work. Great job.

[paulolieuthier]

@mitya57: out of curiosity, why do you consider it usable only after 5.6.1?

[mitya57]

@mitya57: out of curiosity, why do you consider it usable only after 5.6.1?

Because in 5.6.1 I pushed many fixes to the implementation. The most important ones are probably qt/qtbase@9c7f37e and qt/qtbase@a4fac65. There has also been a fix in 5.6.2/5.7 which adds support for multiple tray icons per application.

[palinek]

@mitya57 How can the default implementation be used by 3rd party platform theme?

The QtPlatformTheme privates aren't exported anyhow and the libQtPlatformTheme is built only as a static library.

I made a hack for testing in lxqt/lxqt-qtplugin@16f760d, but this surely isn't the way how to include/use the QtPlatformTheme in 3rd party.

[mitya57]

Oh, right, I didn't think about this. Unfortunately there is no public API at the moment, but as you seem to be using private headers in other places anyway, you can try returning new QDBusTrayIcon from your createPlatformSystemTrayIcon() method (maybe with keeping the check that it's available).

We can try making QDBusTrayIcon public, but I don't think it would be easy because it is currently built as part of static Qt5PlatformSupport library, which gets linked in Qt5XcbQpa which doesn't seem to have any public API at the moment.

[luis-pereira]

@mitya57 We use private headers. But the classes we use are all exported.

Edit: typo

[mitya57]

You now have Qt5PlatformSupport in your target_link_libraries. It is a static library, so all symbols in it can be considered exported. So I think keeping using that library is the only your option for now.

Ubuntu's old platform theme (aka appmenu-qt5) also did that (and it inherited from QGnomeTheme, in a similar way to how you now inherit from QGenericUnixTheme).

[palinek]

You now have Qt5PlatformSupport in your target_link_libraries. It is a static library, so all symbols in it can be considered exported. So I think keeping using that library is the only your option for now.

Ubuntu's old platform theme (aka appmenu-qt5) also did that (and it inherited from QGnomeTheme, in a similar way to how you now inherit from QGenericUnixTheme).

You can't be serious about that ... IMO relying on a completely hidden implementation in QtPlatformTheme which seems to be intended exclusively for the purposes of Qt5XcbQpa is not a good choice:

• we will just blindly hack the include dirs to be able to compile
• we will link to static library which, I believe, is installed unintentionaly (because it is included in the Qt5XcbQPa)

... so our implementation/build will be fragile and can break anytime

[mitya57]

I am not saying that doing that is a good thing, I am just saying that there are no other choices at the moment.

I have just sent a mail about possible ways to solve this to the initial author of D-Bus tray implementation (Shawn Rutledge, who is not on GitHub) CCing you all. Hopefully we'll be able to resolve this problem properly on Qt side.

[paulolieuthier]

Bottom line: it's not "custom code", it's our implementation. Qt's implementation is private and thus not usable. If Qt devs decide to make their implementation public and distributions ship it, we'll definitely use it.

[luis-pereira]

@paulolieuthier I second that. It would be great if Qt devs could make it public.