----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114921/#review47124 -----------------------------------------------------------
Ship it! Yep, as discussed. Looks good, except for one invalid URL. autotests/kfileitemactionstest.cpp <https://git.reviewboard.kde.org/r/114921/#comment33580> This is an invalid way to create a URL from a local file, you must use QUrl::fromLocalFile(). Then again ... it means this whole code really doesn't care about the URL being used, does it? ;-) So you might just as well use "file:///" or whatever. - David Faure On Jan. 9, 2014, 8:15 a.m., Frank Reininghaus wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114921/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2014, 8:15 a.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > ------- > > This patch is a result of the discussion in > http://lists.kde.org/?t=138687009400001&r=1&w=2 > > Currently, KFileItemActions makes the widget that is set with > setParentWidget(QWidget*) the parent not only of any dialogs that are shown > (as advertised by the API docs), but also of the created actions. > Nonetheless, KFileItemActions remembers pointers to all created actions and > deletes them in the destructor. This can cause problems if the widget is > deleted before the KFileItemActions instance - the destructor will then try > to delete dangling pointers and cause a crash. > > This problem can be fixed by making KFileItemActions the parent of the > actions. This also makes the code a bit simpler because the m_ownActions > member is not needed any more. > > In fact, this issue is the cause of crashes in Dolphin (see > https://bugs.kde.org/show_bug.cgi?id=259089). I still think that we don't > really have to change it in kdelibs 4.x because the problem can be worked > around (https://git.reviewboard.kde.org/r/114440/ , which isn't pushed yet > because it turns out that there is still another source of crashes in the > problematic Dolphin use case). > > > Diffs > ----- > > autotests/CMakeLists.txt 2868327 > autotests/kfileitemactionstest.cpp PRE-CREATION > src/widgets/kfileitemactions.cpp eee2ebe > src/widgets/kfileitemactions_p.h 9f9a701 > > Diff: https://git.reviewboard.kde.org/r/114921/diff/ > > > Testing > ------- > > New unit test crashes with master, and passes if the patch is applied. > > Existing kio unit tests pass on my system (except for kiocore-kacltest, but I > believe that the failure is unrelated to this patch). > > > Thanks, > > Frank Reininghaus > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
