<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/121197/">https://git.reviewboard.kde.org/r/121197/</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 21st, 2014, 4:21 nachm. CET, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/121197/diff/1/?file=329330#file329330line1010" style="color: black; font-weight: bold; text-decoration: underline;">src/kstartupinfo.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QByteArray KStartupInfo::createNewStartupId()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1010</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">qt_x_user_time</span> <span class="o">==</span> <span class="mi">0</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<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;">Should this not rather be ::getTimestamp() unconditionally?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is supposed to be a hint when the application was triggered, not when there was the last interaction in the calling client.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">::appUserTime() might be any random value in the past, thus pollute the FSP heuristics.</p></pre>
</blockquote>
<p>On November 21st, 2014, 7:09 nachm. CET, <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;">you have a point. I thought the current usage assumed that e.g. when Krunner starts an application that it creates the ASN after the event which triggered it. The event contains a timestamp and Qt should(TM) update the app user time accordingly. From reading the code that is currently not the case (I checked in Kickoff), but could easily be changed to be correct and is something I wanted to look into. It would be the correct timestamp in that case and doesn't trigger another roundtrip to X during starting an application.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">additional idea: changing createNewStartupId() to createNewStartupId(quint32 timestamp = 0)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It would allow users having the up to date timestamp to pass it in and in all other cases to just use the one from getTimestamp.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On November 21st, 2014, 11:44 vorm. CET, Martin Gräßlin 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 KDE Frameworks, kwin and Plasma.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Nov. 21, 2014, 11:44 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwindowsystem
</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;">KStartupInfo::createNewStartupId is supposed to return a new
id with a properly setup user timestamp. But if QX11Info::appUserTime
returns 0 this was included in the id and cannot be considered a
properly setup user timestamp. An app user time of 0 is the case
for klauncher. If an application uses this timestamp of 0 passed from
the startup notification it causes problems with passing focus to it
from KWin. The application sets the timestamp in the
_NET_WM_USER_TIME property and the value "0" has the special meaning
of requesting to not be initially mapped. KWin honors that and doesn't
focus the window. An application is not supposed to interpret a 0 time
stamp passed through the startup notification as a special value to get
the current time. It is totally fine to expect that it gets a proper
value passed. Thus one cannot blame applications for not interpreting
this value accordingly. An example for an application passing the 0
to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
session through e.g. the launcher results in KWin not passing focus
to it and instead demands attention. This is clearly wrong and not
intended.
The solution in this change is to get the current timestamp from
the X server in case the appUserTime is 0. This fixes the described
problem with Firefox: it now gets properly focused on starting
through a launcher.</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>autotests/kstartupinfo_unittest.cpp <span style="color: grey">(f4b607d2a21da7dcf32434c00b2bdeeaf401ee65)</span></li>
<li>src/kstartupinfo.cpp <span style="color: grey">(03418fce1a154ea388d0f65665dc0c9724a5623a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121197/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>