Implement a cancelled() signal in ReadOnlyArchiveInterface to fix cancellation of password dialogs
Review Request #123967 - Created May 31, 2015 and submitted
Implement a cancelled() signal in ReadOnlyArchiveInterface. This signal is emitted in CliInterface when the user cancels a password dialog. The signal is connected to a slot in Job which sets KJob::error to KJob::KilledJobError.
The callers (Part and BatchExtract) are modified to do nothing if the error code is KJob::KilledJobError when running ListJobs/ExtractJobs/AddJobs/DeleteJobs. This fixes an issue where the user cancels the "Enter password" dialog and the UI keeps being blocked and/or error messageboxes appear.
The commit ee006cb6a83e6cbd01a2dc459b8b0c8be664c05e in the frameworks branch should be reverted before applying this diff, since this is meant to be an improved replacement of that commit.
Opened a password-protected zip archive, clicked on a file and cancelled the dialog. Opened a password-protected zip archive, selected a file for extraction and cancelled the password dialog. Opened a password-protected rar archive, and cancelled the password dialog. In all cases, the UI is not blocked after cancelling the dialog and no message boxes appear.
Before commenting on the smaller details in the code, a few more pressing comments:
- You are checking for
ReadOnlyArchiveInterface::Cancelledin several parts which only interact with the
Joblayer, which looks like an oversight.
- In certain cases in
handleLine()), manually emitting
finished()in addition to calling
failOperation()causes crashes. See commit c7af2d6372ea2b222e1af223b3fd53cdeb1f3c2b.
- Have you seen the follow-up I sent to my own email in kde-commits? http://article.gmane.org/gmane.comp.kde.cvs/1420983 -- it contains a suggestion for a different approach that might avoid some of the pitfalls here.
Modify summary and description to reflect changes in diff.
Set the error code to KJob::UserDefinedError (=100) in Job::onError(), not to 1 which corresponds to KJob::KilledJobError. Otherwise, typing the wrong password gives no error.
Modify BatchExtract to not show errorbox when error() is KJob::KilledJobError.
However, there is still an inconsistency: When BatchExtracting a rar-archive and cancelling the password box there is an errorbox with the message: "Extraction failed: Incorrect password", but when BatchExtracting a zip-archive and cancelling there is no errorbox.
Revision 5 (+31 -12)
Are you sure this is going to work? If you have an error message (so
job->error() != 0), you are reaching the
ifclause regardless of the error code (
UserDefinedError), and then the
else ifonly works if
job->error()returns 0 (no error).
It might even be what's causing the inconsistency you described.