Review Request 122220: Don't show two progress dialogs for PasteJob

[Martin Klapetek]

> On Jan. 23, 2015, 3:40 p.m., David Faure wrote:
> > src/widgets/pastejob_p.h, line 58
> > <https://git.reviewboard.kde.org/r/122220/diff/1/?file=344358#file344358line58>
> >
> >     Correct, but very unusual for kio jobs, so please add a comment, like this:
> >     
> >     // Note: never KIO::getJobTracker()->registerJob here
> >     // The progress information comes from the underlying job (so we don't have to forward it here).
> >     
> >     
> >     
> >     (This way, if we ever find a reason why this job should be registered and not the subjob, we'll know that it's possible, provided that we forward progress info here).
> >     
> >     One reason could be - does everything work correctly when the user clicks Cancel in the progress dialog. I think so, but I didn't test (mostly tested this with unittests).
> >     
> >     Anyway - I want the comment in here so that if anyone uses this as the starting point for a new job, then that person remembers to think about whether or not to call registerJob ;-)

> does everything work correctly when the user clicks Cancel in the progress dialog

Clicking cancel on the subjob does cancel it properly (the file will stay half-written on copy, but I'm not sure if that's wanted or not), but Plasma will not spawn any notification while it does when cancelling the PasteJob (although it says "Copying Finished" which is kinda wrong anyway). But that's a bug in the Plasma applet, so I'll have a look at that too.


- Martin


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


On Jan. 23, 2015, 3:13 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122220/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2015, 3:13 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 342276
>     https://bugs.kde.org/show_bug.cgi?id=342276
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> PasteJob creates a copy/move job and passes its flags to it. These jobs then register with the progress dialog, but so does the original PasteJob and then there are two progress bars/dialogs.
> 
> This moves the registration to the job tracker only to the pasteMimeDataImpl job which does not take the flags of the parent job. I'm not sure if it makes sense there though. Alternatively it could be just removed.
> 
> 
> Diffs
> -----
> 
>   src/widgets/pastejob.cpp a0ce3ac 
>   src/widgets/pastejob_p.h f5e68d3 
> 
> Diff: https://git.reviewboard.kde.org/r/122220/diff/
> 
> 
> Testing
> -------
> 
> Copying and moving files no longer spawns two progress dialogs.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150123/0ddab4fe/attachment.html>