<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126016/">https://git.reviewboard.kde.org/r/126016/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 11th, 2015, 7:14 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On November 11th, 2015, 10:08 a.m. UTC, <b>Eike Hein</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">+1, I want to get rid of hacks, not pile on them</p></pre>
</blockquote>
<p>On November 11th, 2015, 2:02 p.m. UTC, <b>Johan Ouwerkerk</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On November 11th, 2015, 3:37 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">yeah sure, this was not meant as a "blocking" comment. I think the change should go in, but leave the decision to Eike.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I btw. already started working on the improvement by proposing a new addition to the NETWM spec and implementing it in KWindowSystem.</p></pre>
</blockquote>
<p>On November 14th, 2015, 3:43 p.m. UTC, <b>Johan Ouwerkerk</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Eike: what's your verdict? To ship or not to ship?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(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.)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now that my developer account has been approved (that took a few weeks), let's revisit this:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">@Eike: should I push to master or not?</p></pre>
<br />
<p>- Johan</p>
<br />
<p>On November 10th, 2015, 6:54 p.m. UTC, Johan Ouwerkerk wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Johan Ouwerkerk.</div>
<p style="color: grey;"><i>Updated Nov. 10, 2015, 6:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch can be pulled from the kcmshell5-task-url-fixes branch at: git@github.com:cmacq2/plasma-workspace.git</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Built with kdesrc-build, and tested using: <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">plasmawindowed org.kde.plasma.icontasks</code>.
I checked the change works as expected by running <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">which kcmshell5</code> style as well as <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">kcmshell5 style</code>: the icon of the window matches that in system settings (as expected).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libtaskmanager/taskitem.cpp <span style="color: grey">(3b2a4188fc8ed087a331999aee279ecd919c628e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126016/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>