[PATCH] Take care also of the KJob::finished signal in KPart - it also fixes Ark's BUG 187538

David Faure faure at kde.org
Tue Apr 7 10:49:22 BST 2009

On Monday 06 April 2009, Kevin Ottens wrote:
> So what would happen if we made EmitResult by default is 
> that in most cases of user cancellation, the user would then get a dialog 
> claiming that the job failed... which isn't true, and isn't desirable to put 
> into the face of the user (he knows he killed the task, he clicked on the 
> button).

This worked in KDE3 by emitting error, and the showErrorDialog method
wouldn't do anything if the error was ERR_USER_CANCELED. That meant
one special case in the GUI code, but no special case in the application-logic code,
which I think is good because there is much more app-logic related to jobs
than duplications of the "show this error" mechanism.

I hadn't realized that the default for kill() had changed in kde4. This sounds nasty
(if only because the porting might have overseen that)...
I guess it's too late to change our mind about this now :(
Well. Not sure.
IMHO this introduces a bug that every bit of code that expects result() to be emitted
in all cases where the job is killed [*] is now running under a false assumption, exactly
like this kpart code, but there's probably a lot more like it. I think we should make
the "cancel" button call kill(EmitResult).

[*] The point of Quietly is for when the app code changes its mind and kills the job
and doesn't want slotResult to be called -- but the app code has full control over that.
In all cases where the job is killed independently, then it has to be notified,
and that's exactly what result() is about, and the reason why Cancel should emit result.

> That's why EmitResult is a possibility but not the default, and shall not be 
> the default.

It was, in kde3...

David Faure, faure at kde.org, sponsored by Qt Software @ Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).

More information about the kde-core-devel mailing list