Review Request 122500: Don't delete widgets we don't own when changing styles

[Hugo Pereira Da Costa]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122500/#review75715
-----------------------------------------------------------


Although I have no objection against the change, I must admit I don't understand what's wrong with the current code, nor the actual description of the patch. 
"registerWidget may take an existing widget as a parameter. If so, we
don't want to delete it"
if I understand my own code right, _widget is not the argument passed as the registerWidget method. It is an internal member, created parentless, at first accepted call to registerWidget. So as such it is not 'explicitly' owned by anyone, and implicitly owned by us. 
And then, what is wrong with deleting it in our destructor ? 
is it because, though parentless it might get deleted elsewhere ? 
or because of a thread issue ? 
What do I miss ? 
(PS: the reason behind this interal _widget member, is that you can not track palettechanged events on a widget, via event filter, once you set it your own 'altered' palette: it won't recive these events anymore. 
So: eventFilter must be installed on either qApp (which is then getting the event for _every_ widget, for which we did not alter the palette, which is quite a lot), or on a widget for which we are sure the pallette is not altered. Hence: our own).


kstyle/breezepalettehelper.cpp
<https://git.reviewboard.kde.org/r/122500/#comment52340>

    Shouldn't this be "as this isn't a QWidget"
    (since 'this' actually is a qobject)


- Hugo Pereira Da Costa


On Feb. 9, 2015, 2:33 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122500/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2015, 2:33 p.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Repository: breeze
> 
> 
> Description
> -------
> 
> Don't delete widgets we don't own when changing styles
> 
> registerWidget may take an existing widget as a parameter. If so, we
> don't want to delete it when our paletteHelper is deleted for example if
> we change style.
> 
> 
> Diffs
> -----
> 
>   kstyle/breezepalettehelper.cpp 31c32c3 
> 
> Diff: https://git.reviewboard.kde.org/r/122500/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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