Review Request 118269: Fix crash when showing the confirmation dialog for trash/delete operations

[Frank Reininghaus]

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

(Updated June 4, 2014, 8:58 a.m.)


Status
------

This change has been discarded.


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/20140604/bf669c58/attachment.html>