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

[Dawit A.]

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.