Review Request 128473: Avoid recursive calls to QPlatformTheme::createPlatformSystemTrayIcon()

[Martin Tobias Holmedahl Sandsmark]

> 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?

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


-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160719/657d0563/attachment.html>