Review Request 118269: Fix crash when showing the confirmation dialog for trash/delete operations
David Faure
faure at kde.org
Sun Jun 1 15:00:59 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118269/#review58902
-----------------------------------------------------------
That makes the setWindow call (as called by konq_operations.cpp) useless, though.
This is related to the line in jobuidelegate.cpp which says
QWidget *widget = job() ? window() : NULL; // ### job is NULL here, most of the time, right?
Clearly we need a better way to pass the QWidget* parent to the askDeleteConfirmation() method, it's not working on either end (because no job).
But the API (JobUiDelegateExtension) is in KIOCore, so no QWindow nor QWidget there.
I think we need to use a member variable in KDialogJobUiDelegate in addition to setting it in KJobWidgets.
- David Faure
On May 22, 2014, 9:39 p.m., Frank Reininghaus wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118269/
> -----------------------------------------------------------
>
> (Updated May 22, 2014, 9:39 p.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Bugs: 334648
> https://bugs.kde.org/show_bug.cgi?id=334648
>
>
> Repository: kjobwidgets
>
>
> Description
> -------
>
> Currently Dolphin (and probably anything else that can delete files using KIO) crashes when it's supposed to show the confirmation dialog.
>
> The problem is that KonqOperations::askDeleteConfirmation() sets up a KIO::JobUiDelegate object which has no associated job, and calls its setWindow(QWidget*) method before calling the actual askDeleteConfirmation() function.
>
> KIO::JobUiDelegate::setWindow(QWidget*) then calls the setWindow function of the base class, KDialogJobUiDelegate::setWindow(QWidget*), which then crashes because it contains the line
>
> Q_ASSERT(job())
>
> and there is no job.
>
> The problem does not exist in KDE SC 4.x - the code went through a big refactoring in
>
> https://git.reviewboard.kde.org/r/111081/
>
> which added the assert.
>
> Just removing the assert won't help because the function then calls
> KJobWidgets::setWindow(KJob *job, QWidget *widget)
> which dereferences the job, i.e., we get a segfault instead.
>
> This patch removes the assert and wraps the function in an "if (job())" block instead. I'm not entirely sure if that is the correct solution though - any feedback is welcome. Alternatively, one could move the if-check to the child class, i.e., to KIO::JobUiDelegate::setWindow(QWidget*), if this is the only valid use of a KDialogJobUiDelegate without a job.
>
> Or maybe it does not make much sense at all to have the askDeleteConfirmation function, which is probably always called before any job is set up, in a KDialogJobUiDelegate subclass? Changing that would probably require more intrusive changes though.
>
>
> Diffs
> -----
>
> src/kdialogjobuidelegate.cpp fb4c99a
>
> Diff: https://git.reviewboard.kde.org/r/118269/diff/
>
>
> Testing
> -------
>
> Fixes the crash for me, and the confirmation dialog works as expected (i.e., the user can choose if the file should really be deleted/trashed or not).
>
>
> Thanks,
>
> Frank Reininghaus
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140601/4dae67bd/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list