Review Request 126016: fix: properly recognise Plasma 5 KCM modules (wmClass=kcmshell5)

[Johan Ouwerkerk]

> On Nov. 11, 2015, 7:14 a.m., Martin Gräßlin wrote:
> > personal comment from X world: this is horrible, horrible ;-) What we should try is to make the desktop file available to the window. With KF 5.16 we will have all that's needed available. Let's try to improve this in Plasma 5.5 and scratch the code completely.
> 
> Eike Hein wrote:
>     +1, I want to get rid of hacks, not pile on them
> 
> Johan Ouwerkerk wrote:
>     > With KF 5.16 we will have all that's needed available. Let's try to improve this in Plasma 5.5 and scratch the code completely.
>     
>     Great and I agree that the code is not great --all those wasted service lookups and subtly broken caching of the answer-- but: where is this alternative code that fixes everything? ;) Right now, I think this change amounts to a trivial fix (just one forgotten case in the if-clause) to an existing 'feature'/workaround that has a fairly immediate benefit (something works again) and little cost: it's hardly a new one.
> 
> Martin Gräßlin wrote:
>     yeah sure, this was not meant as a "blocking" comment. I think the change should go in, but leave the decision to Eike.
>     
>     I btw. already started working on the improvement by proposing a new addition to the NETWM spec and implementing it in KWindowSystem.
> 
> Johan Ouwerkerk wrote:
>     Eike: what's your verdict? To ship or not to ship?
>     
>     (In the case of shipping it: please note that I do not have commit access (just a KDE identity account) so please pull these changes because I cannot commit them.)

Now that my developer account has been approved (that took a few weeks), let's revisit this:

@Eike: should I push to master or not?


- Johan


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


On Nov. 10, 2015, 6:54 p.m., Johan Ouwerkerk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126016/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2015, 6:54 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Previously the taskmanager library contained a special case logic for windows of KDE-4 KCM modules (only).
> These modules were recognised by finding wmClass=Kcmshell4.
> This logic is extended to cover kcmshell5 windows as well, meaning that KCMs written for Plasma 5 are properly recognised now.
> The net benefit is that these KCMs are displayed in the task manager with their proper KCM program icons.
> 
> This patch can be pulled from the kcmshell5-task-url-fixes branch at: git at github.com:cmacq2/plasma-workspace.git
> 
> 
> Diffs
> -----
> 
>   libtaskmanager/taskitem.cpp 3b2a4188fc8ed087a331999aee279ecd919c628e 
> 
> Diff: https://git.reviewboard.kde.org/r/126016/diff/
> 
> 
> Testing
> -------
> 
> Built with kdesrc-build, and tested using: `plasmawindowed org.kde.plasma.icontasks`.
> I checked the change works as expected by running `which kcmshell5` style as well as `kcmshell5 style`: the icon of the window matches that in system settings (as expected).
> 
> 
> Thanks,
> 
> Johan Ouwerkerk
> 
>

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