Review Request: Wait for All Ignored Windows to Close in Controller

[David Narváez]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103858/#review10419
-----------------------------------------------------------


Ping

- David Narváez


On Feb. 3, 2012, 1 p.m., David Narváez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103858/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 1 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> This addresses bug 291075.
>     http://bugs.kde.org/show_bug.cgi?id=291075
> 
> 
> Diffs
> -----
> 
>   plasma/desktop/shell/controllerwindow.cpp 6f3064f 
> 
> Diff: http://git.reviewboard.kde.org/r/103858/diff/diff
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> David Narváez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120208/303851b7/attachment.html>