> On Sept. 17, 2014, 12: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? > > Martin Gräßlin wrote: > > 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.
> QSystemTrayIcon::isSystemTrayAvailable is currently always return true in your QPT. Possibly fixable, at which point the setLegacyMode can have a if () before it tries doing anything. I made a rushed patch as we need something soonish. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120243/#review66738 ----------------------------------------------------------- On Sept. 17, 2014, 12: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, 12: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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
