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

[David Faure]

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



src/widgets/pastejob.cpp
<https://git.reviewboard.kde.org/r/122220/#comment51713>

    Not needed. This is up to the subjob itself. It in fact already does so, since we don't pass HideProgressInfo to it.
    
    (cf putDataAsyncTo in paste.cpp)



src/widgets/pastejob_p.h
<https://git.reviewboard.kde.org/r/122220/#comment51712>

    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 ;-)


Thanks for pinpointing the issue!

- David Faure


On Jan. 23, 2015, 2: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, 2: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/6a6321ac/attachment-0001.html>