<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/129833/">https://git.reviewboard.kde.org/r/129833/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 15th, 2017, 3:32 a.m. CET, <b>Christoph Feck</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;">From what I see, the content widget is not owned by the tool tip widget. What happens if I delete the tool tip widget first, then delete the content widget? Does deleting automatically break the connections to the now non-existing target?</pre>
</blockquote>
<p>On January 15th, 2017, 11:48 a.m. CET, <b>Elvis Angelaccio</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;">Good point. Actually I just realized that the tooltip does take ownership, so what happens is a double delete :/
However, this is unrelated from this patch (it crashes even if I remove the new connect()).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We could fix this with another patch or maybe we could just warn in the apidox to not delete the content after deleting the tooltip...</p></pre>
</blockquote>
<p>On January 17th, 2017, 3:37 a.m. CET, <b>Christoph Feck</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;">Hm, from looking at the API, you would not assume transferring of ownership. It is the adding to the layout that transfers?
Anyone who already uses the class probably has noticed this, so I would suggest to keep the behavior and document it. Later we could add a call that allows to cancel the ownership. Maybe this might sense for widgets that need to be shown often, but are complex to reinitialize.</pre>
</blockquote>
<p>On January 17th, 2017, 6:48 p.m. CET, <b>Elvis Angelaccio</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;">Yes, it's because of the layout.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What we could also do is storing the old parent (before adding to the layout) and restoring it later. lxr says no one is using this class yet, so we might be still in time...</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;">lxr only scans KDE sources, so theoretically, someone else could already use the class. Practically, you are right, and I will accept either solution you propose.</pre>
<br />
<p>- Christoph</p>
<br />
<p>On January 14th, 2017, 7:54 p.m. CET, Elvis Angelaccio 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 and Christoph Feck.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated Jan. 14, 2017, 7:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwidgetsaddons
</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;">Otherwise the tooltip will be visible but empty.</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;">The new test case reproduces this issue.</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>autotests/ktooltipwidgettest.h <span style="color: grey">(556d93edd7792736bb7b41761f2d8934395d2f3c)</span></li>
<li>autotests/ktooltipwidgettest.cpp <span style="color: grey">(89124c6b3f16b76a346a8c9c74ff2f9efe0c9e83)</span></li>
<li>src/ktooltipwidget.cpp <span style="color: grey">(979d4f71b5ebf9d0716e12453ecbfc88e48be72d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129833/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>