> 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 > >
