[WebKit-devel] Protected Q_SLOTS in KWebPage

[Dawit A.]

On Monday 12 October 2009 17:07:11 Urs Wolfer wrote:
> On Monday 12 October 2009 05:03:01 Michael Howell wrote:
> > On Sunday 11 October 2009 05:22:30 you wrote:
> > > Michael, do you think it makes sense to have these virtual slots there?
> > > Or should the moved to the d-ptr and be declared with Q_PRIVATE_SLOT?
> > > If they stay there, I'm not sure about the name. Is it common to call
> > > them "slotX"? Also they should be documented.
> > >
> > > Bye
> > > urs
> >
> > slotUnsupportedContent is KDE-integration related. It uses KDE's dialogs
> > instead of the default Qt ones. It can't be private, because WebPage
> >  overloads it to use the KPart one instead. The only thing we can do is
> >  rename it.
> 
> I'm not sure, how does Qt name such protected slots? Is there a convention?
> 
> > I'm not sure why the downloadRequest ones are protected. They should
> >  probably be private.

Well not really... Actually this is something I completely missed when doing 
the recent clean up. KWebPage should not do its own signal/slot connection. 
Since its intent is to provide KDE integration where it makes sense, these 
protected slots, specially slotDownloadRequest, should be renamed and provided 
as a public member function or slot. That way either class inheriting from 
KWebPage or using directly can connect/call this public function as they see 
fit. IOW, the signal/slot connections as well as the call to 
setForwardUnsupportedContent must to be removed from KWebPage's constructor.

I also personally prefer completely getting rid of slotUnsupportedContent 
since it does not provide anything except asking the user for filename using 
KParts::BrowserRun which really does not belong there IMHO. In fact that would 
mean we can get rid of a couple of more functions in KWebPage including the 
overloaded slotDownloadRequest...

> >  Actually, it seems some information has lost sync. slotUnsupportedContent
> >  in KWebPage, and handleUnsupportedContent in WebPage?
> No idea here.. What do you suggest? Make them privat? Any idea in what
>  commit it got messed up?

That would be my mistake... I actually started to rename those functions and 
changed my mind, but appartently forgot to put the one in WebPage back to what 
it was. It is fixed now...