> On July 5, 2013, 2:23 p.m., David Faure wrote: > > kio/kio/scheduler.cpp, line 766 > > <http://git.reviewboard.kde.org/r/111335/diff/2/?file=167766#file167766line766> > > > > Please don't put this code in scheduler.cpp > > > > I'm trying to properly split core and gui aspects of KIO in frameworks, > > and scheduler is definitely core, while kmessagebox is definitely not. > > > > Please find a way to separate the two. > > Dawit Alemayehu wrote: > So how were you planning to separate the core and gui aspects in > frameworks? Without this patch KIO::Scheduler will still be linking against > gui libraries because of its use of KIO::Slave. Perhaps if I know how you > were planning to perform the split, I could follow the same approach to > resolve this issue and it would be one less thing you have to deal with. >
I'm separating core/gui stuff for jobs using delegates and delegate extensions and factories. (the trick is that the kiowidgets library can register stuff using code that runs automatically, just by linking to the library, see Q_CONSTRUCTOR_FUNCTION in kio/jobuidelegate.cpp) But nothing is done yet for the messagebox stuff in Slave. So this is good timing, let's come up with a solution (which we could probably apply to both branches). Context: SlaveInterface::messageBox() is called by kioslaves, in the application process. Let's brainstorm. I can think of 3 solutions on top of my head: 1) Defining an interface in kiocore and implementing it in both libs. The core implementation would have to return "Cancel" every time, for lack of a possibility to interact with the user. Apps could still reimplement that interface to give predefined answers. 2) Propagating the call up to the job, which can then use the delegate mechanism for showing the messagebox (with again a canned reply for core-only code) 3) Delegating the messagebox to a separate process, e.g. kuiserver. This is the KDE3 solution, actually. The commit that removed that (8d6f7d340e0) says that modal messagebox were blocking kuiserver. But we could use non-modal messageboxes instead. Either with a blocking dbus call (using dbus transactions in kuiserver), or with real async everywhere. Problem: what if kuiserver isn't available. Or what if you wanted a core-only command-line tool which would not interact with the user. Any other ideas? Any input on the above possibilities? I kind of like number 2, to reduce the number of interfaces being used to call gui stuff from kiocore. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111335/#review35634 ----------------------------------------------------------- On July 3, 2013, 12:19 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111335/ > ----------------------------------------------------------- > > (Updated July 3, 2013, 12:19 p.m.) > > > Review request for kdelibs. > > > Description > ------- > > The attached patch addresses one of the oldest bugs in KIO. Due to the > muti-process nature of KIO, if any of the ioslaves encounter something that > requires user input, the user might end up getting prompted multiple times. > The best example of this is SSL error warnings sent to the client by > kio_http. The patch completely resolves this problem using the same approach > as kpasswdserver, but without the need for an additional kded process. > > > This addresses bugs 154100 and 265228. > http://bugs.kde.org/show_bug.cgi?id=154100 > http://bugs.kde.org/show_bug.cgi?id=265228 > > > Diffs > ----- > > kio/kio/scheduler.h 04edb40 > kio/kio/scheduler.cpp 802f8b8 > kio/kio/scheduler_p.h d68f645 > kio/kio/slaveinterface.h 4bfcec8 > kio/kio/slaveinterface.cpp aa0fc44 > kio/kio/slaveinterface_p.h e31ec5e > > Diff: http://git.reviewboard.kde.org/r/111335/diff/ > > > Testing > ------- > > Visit a site that throws up SSL warnings and causes KIO to show more than one > error dialog. > > > Thanks, > > Dawit Alemayehu > >
