<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112185/">http://git.reviewboard.kde.org/r/112185/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Many thanks for taking care of this.</pre>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/112185/diff/1/?file=183830#file183830line656" style="color: black; font-weight: bold; text-decoration: underline;">tier1/kwindowsystem/src/kmanagerselection.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; ">xcb_window_t KSelectionWatcher::owner()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">651</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">// qDebug() << "handling message";</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">649</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">owner</span><span class="p">()</span> <span class="o">==</span> <span class="n">cm_event</span><span class="o">-></span><span class="n">data</span><span class="p">.</span><span class="n">data32</span><span class="p">[</span> <span class="mi">2</span> <span class="p">])</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">that's still a weird if() then, if it does nothing in the body.
Why not just call owner(); ?</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On August 21st, 2013, 1:33 p.m. UTC, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and David Faure.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Aug. 21, 2013, 1:33 p.m.</i></p>
<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;">Re-enable unit test for kmanagerselection
The testLostOwnership is set to expected fail as that one used
X event queue functionality not available in XCB.
The other tests are working.
Always emit the claimedOwnership() signal if we have the selection
There was a logical error introduced during the porting to XCB. There
is no requirement for the old owner to destroy its selection window.
Which means we should not emit the failed signal if the window has not
been destroyed in time.
This is especially important for the situation that another Selection
Owner of the same X client takes over as the SelectionClear event is
not delivered in that case and thus the destroy never happens and the
new owner thinking it failed.
The timer is in fact only relevant for the case that the kill option
is set. We want to give the old owner the time to clean up itself
instead of forcefully killing (e.g. window manager --replace).
Filter out events only relevant to the KSelectionOwner
No need to pass the events about the selections we are monitoring to Qt.
Ensure the KSelectionOwner destroys its window in the dtor
Release() might not destroy the window, so better make sure that
the window gets destroyed.
Re-add code for client message handling in KSelectionWatcher
While the code looks like being no-op, it is in fact not, because in
one path it calls owner() which might update the selection owner and
emits newOwner signal. Updating the owner information has to happen
in response to handling the client message.
Need to declare metatype for xcb_window_t
This fixes not delivered signals - e.g. newOwner.</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>tier1/kwindowsystem/autotests/CMakeLists.txt <span style="color: grey">(614b852)</span></li>
<li>tier1/kwindowsystem/autotests/kmanagerselectiontest.h <span style="color: grey">(15858dc)</span></li>
<li>tier1/kwindowsystem/autotests/kmanagerselectiontest.cpp <span style="color: grey">(887cb3f)</span></li>
<li>tier1/kwindowsystem/src/kmanagerselection.h <span style="color: grey">(8df8ead)</span></li>
<li>tier1/kwindowsystem/src/kmanagerselection.cpp <span style="color: grey">(d7453c8)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112185/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>