<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/103858/">http://git.reviewboard.kde.org/r/103858/</a>
     </td>
    </tr>
   </table>
   <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/103858/diff/1/?file=48650#file48650line497" style="color: black; font-weight: bold; text-decoration: underline;">plasma/desktop/shell/controllerwindow.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; ">void ControllerWindow::closeIfNotFocussed()</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">497</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">widget</span><span class="o">-></span><span class="n">property</span><span class="p">(</span><span class="s">"DoNotCloseController"</span><span class="p">).</span><span class="n">isNull</span><span class="p">())</span> <span class="p">{</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">497</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">widget</span><span class="o">-></span><span class="n">property</span><span class="p">(</span><span class="s">"DoNotCloseController"</span><span class="p">).</span><span class="n">isNull</span><span class="p">()</span><span class="hl"> </span><span class="o"><span class="hl">&&</span></span><span class="hl"> </span><span class="n"><span class="hl">m_ignoredWindowClosed</span></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;">m_ignoreWindowClosed is true when a window with the DoNotCloseController property is closed.

so this change means that only if such a window has closed and the new window does not have a DoNotCloseController tag, then the controller will close. 

when, exactly, is that condition met?

basically, this renders it such that opening ANY window in the plasma-desktop process will not close the controller, and that is obviously not what is wanted.

a proper fix is probably to check that none of the parent windows also have this property.</pre>
</div>
<br />



<p>- Aaron J.</p>


<br />
<p>On February 3rd, 2012, 1 p.m., David Narváez wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Plasma.</div>
<div>By David Narváez.</div>


<p style="color: grey;"><i>Updated Feb. 3, 2012, 1 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;">This patch makes the controller wait for the ignored window to actually close. In bug 291075 you can see what happens when you don't: the Browse window pops, the Controller queries for the Active Window, the Active Window is not tagged, so the Controller kills all of its children, including the KIconDialog itself.

Even after this patch there's still a way to trigger this bug: since we are only managing a boolean flag, you can chain something like

openWindowWithTag() -> openWindowWithoutTag() -> openWindowWithTag()

and then close the last window, which will close the controller. I think the proper way to manage this is to have a counter of active tagged windows, and only close when it is 0.</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;">1) Click on the Activity Bar plasmoid
2) Change the icon of an activity
3) Use Custom Icon
4) Browse
5) Select an SVG file

Before this patch, Plasma would crash on step 4. Notice that, on my setup, selecting a PNG file would cause the controller to close (no crash, just close) and the icon wouldn't change. I think I remember being able to set a PNG file before, so please let me know if this is a problem caused by this patch.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=291075">291075</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>plasma/desktop/shell/controllerwindow.cpp <span style="color: grey">(6f3064f)</span></li>

</ul>

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




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








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