DevHeads.net

Review Request: Set the parent widget in KIO::SlaveInterface::messageBox

Review request for kdelibs.

Description
This patch sets the top level window that should be used when displaying message boxes in KIO::SlaveInterface. This is the first step into eliminating those multiple message boxes that are popped up because of the out-process design of KIO. Note that this is not the actual resolution, but the first step.

Diffs
kio/kio/scheduler.cpp 8d144bb
kio/kio/slaveinterface.h 3cdc2ae
kio/kio/slaveinterface.cpp d2f9f93
kio/kio/slaveinterface_p.h e2ccfe0

Diff: <a href="http://git.reviewboard.kde.org/r/104351/diff/" title="http://git.reviewboard.kde.org/r/104351/diff/">http://git.reviewboard.kde.org/r/104351/diff/</a>

Testing

Thanks,

Dawit Alemayehu

Comments

Re: Review Request: Set the parent widget in KIO::SlaveInterface

By Commit Hook at 04/30/2012 - 19:45

This review has been submitted with commit 0057256b1a0a601346f3a343bbbec81bf2e48fe3 by Dawit Alemayehu to branch KDE/4.8.

- Commit Hook

On March 20, 2012, 7:14 p.m., Dawit Alemayehu wrote:

Re: Review Request: Set the parent widget in KIO::SlaveInterface

By David Faure at 04/30/2012 - 18:24

Ship it!

Oh I see, it's a bug in reviewboard.
The method name in the context (above the diff) was wrong.
It said:

[ void ProtoQueue::removeJob(SimpleJob *job) ]
{
int error;

while in fact the code was from ProtoQueue::createSlave.

Objection withdrawn.

- David Faure

On March 20, 2012, 7:14 p.m., Dawit Alemayehu wrote:

Re: Review Request: Set the parent widget in KIO::SlaveInterface

By David Faure at 04/30/2012 - 16:41

Looks ok, but probably needs rebasing, the code in ProtoQueue::removeJob(SimpleJob *job) looks very different here.

(I think checking for job being null is overzealous)

- David Faure

On March 20, 2012, 7:14 p.m., Dawit Alemayehu wrote:

Re: Review Request: Set the parent widget in KIO::SlaveInterface

By Dawit A at 04/30/2012 - 18:13

Hmm... It applies just fine here on the 4.8 branch and I do not have any other local changes in scheduler.cpp. No idea what that might. As far as the check for job being NULL might indeed be a bit overzealous, but I was simply following the same thing that was done in the else statement right below that code. Well at least that is my story and I am sticking to it. ;)

- Dawit

On March 20, 2012, 7:14 p.m., Dawit Alemayehu wrote: