On Thu, Dec 06, 2007 at 11:05:43AM +0100, Johannes Hofmann wrote:
Hello,
now that was a weird bug! I was experiencing crashes when I had changed the panel size via the right mouse on the search button. The problem is that FltkWidgetView::prepareCreateFltkWidget() was calling fltk::Group::begin(). Therefore after visiting pages with forms, the newly created panel was getting a child of Main instead of TopGroup!
Btw. did I mention that this fltk begin()/end() stuff is evil?
Great you commented this here. Side effects are sometimes very hard to detect and fix. Probably the begin/end pair is less side-effect prone when the GUI is static and built once and in a single function. For dynamic content (as HTML rendering, resizable panels, etc.) I'd also prefer explicit add() calls.
One could expect that Group::replace() should disconnect the new widget properly from its old parent (if there is any). But that's not the case. I wrote a mail to fltk.general about this.
Yes, I'd also expect a reparent. If that's not the case, it should be mentioned in FLTK's docs. Which AFAIS is not the case. <quote> void fltk::Group::replace(int index, fltk::Widget&) Remove the indexed widget and insert the passed widget in it's place. void fltk::Group::replace(fltk::Widget& old, fltk::Widget&) Find the old widget and remove it and insert the new one. If the widget is not a child, the new widget is appended to the end of the list. </quote>
So in the end Main still had a reference to the panel and once it was deleted -- bang.
The patch below uses Group::add() directly in FltkWidgetView::addFltkWidget() instead of FltkWidgetView::prepareCreateFltkWidget().
Committed.
Does anyone remember why FltkWidgetView::prepareCreateFltkWidget() was added in the first place?
I don't know, but while developing dillo's UI I added a similar function to group different combinations of calls until finding one that worked as I wanted. At that time the API wasn't as stable as now, so maybe Sebastian did the same. -- Cheers Jorge.-