Review Request 129849: KToolTipWidget: don't take ownership of the content widget

Elvis Angelaccio elvis.angelaccio at kde.org
Mon Jan 30 23:21:51 UTC 2017



> On Jan. 29, 2017, 10:05 p.m., Aleix Pol Gonzalez wrote:
> > Shouldn't the tooltip be the parent of the widget anyway?

I'd say this is up for discussion, since the api is fairly new. Either we make the tooltip take ownership and we warn about this (to avoid double delete crashes), or we don't and we leave to clients the cleaning task (what this patch does). The latter approach has also the advantage that complex widgets could be used more than once in different tooltips.


- Elvis


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


On Jan. 17, 2017, 10:47 p.m., Elvis Angelaccio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129849/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 10:47 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> -------
> 
> While the layout of the tooltip sets the tooltip as parent of the content widget, we don't really need to take ownership of it. We can just restore the old parent once we are done.
> This prevents a double delete crash if someone (by accident) deletes first the tooltip and then the content.
> 
> 
> Diffs
> -----
> 
>   autotests/ktooltipwidgettest.h 556d93edd7792736bb7b41761f2d8934395d2f3c 
>   autotests/ktooltipwidgettest.cpp 89124c6b3f16b76a346a8c9c74ff2f9efe0c9e83 
>   src/ktooltipwidget.h cde7e66af797c9888dec59ef50831a2260a2c238 
>   src/ktooltipwidget.cpp 544eb74d005bb4c963c8cc6cd8b941c018cf463e 
> 
> Diff: https://git.reviewboard.kde.org/r/129849/diff/
> 
> 
> Testing
> -------
> 
> Test case reproduces the crash, which is fixed by the patch.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170130/e2e12f1f/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list