[WebKit-devel] KDE Webkit status update...

[Dawit A.]

On Sunday 04 October 2009 09:46:32 Dawit A. wrote:
> On Saturday 03 October 2009 17:06:14 Michael Howell wrote:
> > On Saturday 03 October 2009 09:43:40 Dawit A. wrote:
> > > 1.) Add the search bar to KWebView's parent widget layout when it is
> > > first created and call update on the layout. This is a matter of adding
> > > few lines of code to what is now KWebView::searchBar() and changing
> > > that function into a slot that gets invoked when CTRL+F is pressed. I
> > > have done this in my local copy and it works. However, this is not a
> > > robust solution at all! What if the application using kdewebkit wants
> > > to add its own widgets such as a save password bar (KWallet
> > > integration) for example ?
> > >
> > > 2.) For a more robust solution we need to do something like what KHTML
> > >  does. It has a KHTMLViewBar class which manages widgets you want to
> > > add to the view. KHTML then creates two instance of the view bar, one
> > > at the top and bottom, and allow you to add your own widgets to these
> > > bars. This is what KHTML uses to add the password and search bar
> > > widgets to its view. We can do the a similar thing by creating a public
> > > KWebViewBar class and exposing the instances we create through public
> > > member functions, e.g. topViewBar() and bottomViewBar(). This is
> > > probably a better approach in the long run, no ?
> >
> > Either way KWebView messes with its parent widget in unexpected ways.
> > Particularly, ways that QWebView does not.
> 
> Well that is correct statement only for approach #1. It has a very bad
> limitation in that since it uses
>  parentWidget()->layout()->addWidget(QWidget*) to add the widget, it might
>  screw things if the parent object used the addWidget functions in
>  QGridlayout for example ; so that is true for approach #1, but not for
>  approach #2. See below...
> 
> > The way I do it, KWebView acts  as a normal, self-contained widget.
> > Besides, I don't expose internal APIs. I expose a QWidget.
> 
> And that is exactly the problem. Exposing it as a QWidget completely limits
> its functionality. For example, how do you intend to connect the "Find
>  Next" and "Find Previous" signals from the part ? You cannot. You have
>  either one of two options. Either make SearchBar a public api like
>  KWebView and KWebPage and provide a public function to set a search bar in
>  KWebView so that it can connect to the desired signals and correctly
>  search for the necessary text or completely move it out of kdewebkit and
>  put in the webkitpart where it actually belongs. Truth be told there is no
>  reason for kdewebkit as a rendering engine to provide search widgets at
>  all. Search widgets are a function of the application. That is the source
>  of the problem.
> 
> > I'm surprised that KHTMLWidget messes with it's parents. They have
> > control over how KHTML draws, so they can put the find bar inside it.
> 
> You completely misunderstood the point I was attempting to make or I did
>  not do a good enough job of explaining it. First, so far as I can tell
>  KHTML does not mess with its parent objects at all. In fact it does not
>  even publicly expose the KHTMLViewBar and KHTMLViewBarWidget class. These
>  classes are simply used to manage widgets, such as the search part, that
>  are added to the KHTMLPart as viewbar items, not to the to its rendering
>  view. In retrospect, I realize that doing something like this also belongs
>  in the application, i.e. webkitpart, and not in kdewebkit which is
>  precisely what is done in khtml. However, this still does not resolve the
>  issues with the way the searchbar widget is now exposed to external
>  applications in kdewebkit ; so here is another more viable and completely
>  clean approach:
> 
> #1. Clean up the SearchBar API and make it a publically available widget.
> Perhaps KWebViewSearchBar ??
> 
> #2. Let applications/parts create an instance of this widget if they so
>  choose and connect all the signals they want to the publicly available
>  slots such as show(), findNextMatch(), findPreviousMatch(),
>  setSearchText(const QString&) etc.
> 
> #3. Change the QWidget* searchbar() function in KWebView to
>  setSearchBar(const KWebViewSearch*) so that the KWebView can connect to
>  signals from the searchbar widget and correctly invoke the appropriate
>  member functions during search. Alternatively, we can make searchbar()
>  return a pointer to KWebViewSearch as well to keep it as it currently is.

Note that the above suggestion is only applicable if there is any desire to 
provide a search bar for applications/kparts that might want make use of 
kdewebkit, but not the webkitpart. Otherwise, there is no need for it.