[WebKit-devel] Protected Q_SLOTS in KWebPage

[Dawit A.]

On Thursday 15 October 2009 15:29:33 Dawit A. wrote:
> 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...

Okay, I have committed this change now. I am not sure whether or not I should 
have made the new downloadRequest member function virtual and/or a slot, but 
it should all be cleaned up now...