> On July 19, 2016, 4:02 a.m., David Edmundson wrote: > > KStatusNotifierItemPrivate::setLegacySystemTrayEnabled(bool enabled) > > already has a recursion check added in > > b45544f3d4dd9cb1873b92a609f45a68f5f6e471 (in knotifications) - which > > basically checks if we're using the KDE platform theme (albeit in a > > slightly weird way) > > > > Not saying yours is "worse" but we don't want two fixes in two places. > > > > Could you check you have that patch? and why it doesn't work? > > Martin Tobias Holmedahl Sandsmark wrote: > firstly, as you said, it checks in a weird way, which doesn't work, > that's why I thought it made more sense to fix it in the platform theme > itself which already knows that it is loaded and whether an SNI host is > available. > > (fwiw, qApp->platformName() is not correct either, that's what I thought > was the "proper" way to do it) > > Martin Tobias Holmedahl Sandsmark wrote: > patching around that also leads to no legacy tray icon being created at > all, which is obviously wrong. > > David Edmundson wrote: > Is it wrong? > > If you're logged in to a plasma session with the plasma integration > running it's a valid assumption that people will run Plasmashell. > > Without plasmashell you won't get the legacy tray icons appearing either. > > And if you're running a different shell..you shouldn't be using the > plasma integration in the first place.
The legacy tray icons will work in cases where SNI won't, so breaking that fallback is wrong in my opinion, but I agree it is better than the alternative. Anyhow, this is a mess and the interdependencies on behaviour is bad. IMHO it should be "fixed" in both places, the plasma-integration plugin shouldn't rely on some shaky string magic logic in KSNI not to hang applications. So if the only objection to this patch is that some applications a) under X11 get a legacy tray icon instead of a SNI one if started too early instead of hanging, b) under Wayland might not get a tray icon at all instead of just hanging if started too early, I think this should go in. - Martin Tobias Holmedahl ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128473/#review97506 ----------------------------------------------------------- On July 17, 2016, 8:14 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128473/ > ----------------------------------------------------------- > > (Updated July 17, 2016, 8:14 p.m.) > > > Review request for Plasma. > > > Repository: plasma-integration > > > Description > ------- > > If the status notifier item host is not available, KSNI tries to create a > normal QSystemTrayIcon. > > The plasma platform plugin uses KSNI when it is called to create a > QPlatformSystemTrayIcon. > > So if the status notifier item host for any reason was unavailable, this > would recursively run forever (assuming a turing machine with infinite > memory). > > > Diffs > ----- > > src/platformtheme/kdeplatformsystemtrayicon.h 6825b4d > src/platformtheme/kdeplatformsystemtrayicon.cpp 0e82385 > src/platformtheme/kdeplatformtheme.cpp 5f0407c > > Diff: https://git.reviewboard.kde.org/r/128473/diff/ > > > Testing > ------- > > Now it is possible to run applications that have tray icons with the plasma > platform plugin even when the status notifier item host is down or > unavailable. > > > Thanks, > > Martin Tobias Holmedahl Sandsmark > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
