<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="https://git.reviewboard.kde.org/r/114921/">https://git.reviewboard.kde.org/r/114921/</a>
     </td>
    </tr>
   </table>
   <br />





 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit 7a1543f82d4f4464b204eddbaa439120de0636f2 by Frank Reininghaus to branch master.</pre>
 <br />









<p>- Commit Hook</p>


<br />
<p>On January 9th, 2014, 9:26 p.m. UTC, Frank Reininghaus wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Frank Reininghaus.</div>


<p style="color: grey;"><i>Updated Jan. 9, 2014, 9:26 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">This patch is a result of the discussion in http://lists.kde.org/?t=138687009400001&r=1&w=2

Currently, KFileItemActions makes the widget that is set with setParentWidget(QWidget*) the parent not only of any dialogs that are shown (as advertised by the API docs), but also of the created actions. Nonetheless, KFileItemActions remembers pointers to all created actions and deletes them in the destructor. This can cause problems if the widget is deleted before the KFileItemActions instance - the destructor will then try to delete dangling pointers and cause a crash.

This problem can be fixed by making KFileItemActions the parent of the actions. This also makes the code a bit simpler because the m_ownActions member is not needed any more.

In fact, this issue is the cause of crashes in Dolphin (see https://bugs.kde.org/show_bug.cgi?id=259089). I still think that we don't really have to change it in kdelibs 4.x because the problem can be worked around (https://git.reviewboard.kde.org/r/114440/ , which isn't pushed yet because it turns out that there is still another source of crashes in the problematic Dolphin use case).</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;">New unit test crashes with master, and passes if the patch is applied.

Existing kio unit tests pass on my system (except for kiocore-kacltest, but I believe that the failure is unrelated to this patch).</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/CMakeLists.txt <span style="color: grey">(2868327)</span></li>

 <li>autotests/kfileitemactionstest.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>src/widgets/kfileitemactions.cpp <span style="color: grey">(eee2ebe)</span></li>

 <li>src/widgets/kfileitemactions_p.h <span style="color: grey">(9f9a701)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/114921/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>