Review Request 120243: Remove fallback QSystemTray support

[Martin Gräßlin]

> On Sept. 17, 2014, 2:55 p.m., Aleix Pol Gonzalez wrote:
> > Maybe we want to ifdef it? We won't require Qt 5.4 in a while...
> 
> David Edmundson wrote:
>     It doesn't require Qt5.4.
> 
> Martin Gräßlin wrote:
>     +1 for removing the fallback.
> 
> Luigi Toscano wrote:
>     I guess the ifdef part was about keeping the support until Qt 5.4 is mandatory for Frameworks.
>     
>     
>     I understand you want to get rid of this "legacy" support in Plasma, but forcing it on all application is in my opinion a different story. Isn't there any other solution that removing the code for supporting it on Qt 5.4?
> 
> Martin Gräßlin wrote:
>     > Isn't there any other solution that removing the code for supporting it on Qt 5.4?
>     
>     What could be tried is to use QSystemTrayIcon::isSystemTrayAvailable and not create a QSystemTrayIcon if it returns false.
>     
>     Nevertheless: I'm in favor of removing the code. It was a fallback for the transition to SNI. Nowadays SNI is widely adopted (e.g. Plasma 1 and 5 and Unity) and at the same time support for Xembedd is getting removed more and more and cannot be taken for granted (see initial comment about Wayland).
> 
> Luigi Toscano wrote:
>     Last note on my side: I understand the case of Wayland, but in that case isn't it possible to detect if running under Wayland and skip the fallback?

> I understand the case of Wayland, but in that case isn't it possible to detect if running under Wayland and skip the fallback?

no, not really. The application in question might be connected to an XWayland server and think it's in an X session.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120243/#review66738
-----------------------------------------------------------


On Sept. 17, 2014, 2:07 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120243/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 2:07 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: knotifications
> 
> 
> Description
> -------
> 
> Remove fallback QSystemTray support
> 
> Currently when we make a StatusNotifierItem if it fails we try to create
> a classic X system tray icon via Qt's QSystemTray.
> 
> QSystemTray now (5.4) has a QPT backend that uses KStatusNotifierItem. This
> means it's possible to infinite loop.
> 
> This patch removes the legacy QSystemTray support to avoid that
> happening.
> 
> Downside: it means if a frameworks app is run on a platform without statusnotifier support but still has legacy X system tray support we would now gets no icons, but that would break on Wayland anyway.
> 
> 
> Diffs
> -----
> 
>   src/kstatusnotifieritem.h 7da1451 
>   src/kstatusnotifieritem.cpp 767537d 
>   src/kstatusnotifieritemprivate_p.h 051cc38 
> 
> Diff: https://git.reviewboard.kde.org/r/120243/diff/
> 
> 
> Testing
> -------
> 
> Ran statusnotifiertest (the one in plasma-workspace) with plasma running. Everything is fine.
> Ran without plasma running and nothing happened (without kded5 infinite looping)
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140918/0822d2f1/attachment-0001.html>