- Dillo-dev - lists.mailman3.com

[PATCH]: fix occasional segfault in dw::Table::reallocChildren
by onepoint＠starurchin.org Nov. 17, 2007

Nov. 17, 2007

This patch fixes an occasional segfault when adding columns to a table that contains cells that span multiple rows or columns. This was reported to the list last month by Matthias Franz. I've been bitten by this on several web sites (see the details below), so it's a useful fix. Regards, Jeremy Henty Only in dw2-cur/: Makefile Only in dw2-cur/: Makefile.in Only in dw2-cur/: aclocal.m4 Only in dw2-cur/: autom4te.cache Only in dw2-cur/: config.guess Only in dw2-cur/: config.log Only in dw2-cur/: config.status Only in dw2-cur/: config.sub Only in dw2-cur/: configure Only in dw2-cur/: depcomp Only in dw2-cur/doc: Makefile Only in dw2-cur/doc: Makefile.in Only in dw2-cur/dw: .deps Only in dw2-cur/dw: Makefile Only in dw2-cur/dw: Makefile.in diff -pru -- dw2-ref/dw/table.cc dw2-cur/dw/table.cc --- dw2-ref/dw/table.cc 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/dw/table.cc 2007-11-16 13:02:59.000000000 +0000 @@ -357,7 +357,7 @@ void Table::reallocChildren (int newNumC break; case Child::SPAN_SPACE: if (children->get(children->get(n)->spanSpace.startRow - * newNumCols + + * numCols + children->get(n)->spanSpace.startCol) ->cell.colspanOrig == 0) { colspan0Col = children->get(n)->spanSpace.startCol; Only in dw2-cur/: install-sh Only in dw2-cur/lout: .deps Only in dw2-cur/lout: Makefile Only in dw2-cur/lout: Makefile.in Only in dw2-cur/: missing Only in dw2-cur/test: .deps Only in dw2-cur/test: Makefile Only in dw2-cur/test: Makefile.in Analysis of the bug: A dillo table stores its children in a single vector that is the concatenation of the rows. This means that when the number of columns increases the contents of the vector must be rearranged. The code does this in place, starting at the end and working backwards. When the code reaches a point that corresponds to a multi-row/column cell, it finds a placeholder that stores the row/column of the actual cell. The important point is that the actual cell is stored in the vector *before* any placeholders that refer to it. Therefore the cell has not yet been moved when the code reaches the placeholder (because it works backwards from the end). The code must calculate the index of the actual cell from the row/column in the placeholder based on the *old* number of columns. Unfortunately the code uses the *new* number of columns, so it may retrieve a cell from the wrong place. Sometimes it retrieves a NULL and segfaults when it tries to dereference it. Test cases: <html><head></head><body><table><tbody> <tr><td colspan="2"></td></tr> <tr><td rowspan="2"></td></tr> <tr><td colspan="2"></td></tr> </tbody></table></body></html> [NB: the next one is from Matthias Franz.] <html> <table> <tr></tr> <tr><td colspan="3"></td></tr> <td colspan="3"> </table> </html> Online pages that trigger the bug: <a href="http://legacyeditorial.gettyimages.com/source/search/details_pop.aspx?iid=7…"> <a href="http://www.eweek.com/article2/0,1759,2216435,00.asp"> <a href="http://www.possible-area.org/bad_timing161107.php"> <a href="http://www.eweek.com/article2/0,1895,2216978,00.asp"> <a href="http://www.nostalgia.com/nf_moreinfo.html?sku=10576">

2 1

[patch] delete the phantom widgets
by Johannes.Hofmann＠gmx.de Nov. 15, 2007

Nov. 15, 2007

Hello, here is a patch to really delete the fltk widgets when we leave a page. I hope there are no references to the deleted widgets left, but I'm not 100% sure, so please test it carefully. Also if someone has an idea for a more elegant implementation of FltkWidgetView::removeFltkWidget(), that would be great. Johannes diff --git a/dw/fltkplatform.cc b/dw/fltkplatform.cc --- a/dw/fltkplatform.cc +++ b/dw/fltkplatform.cc @@ -90,6 +90,10 @@ void FltkView::prepareCreateFltkWidget ( void FltkView::addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation) +{ +} + +void FltkView::removeFltkWidget (::fltk::Widget *widget) { } diff --git a/dw/fltkplatform.hh b/dw/fltkplatform.hh --- a/dw/fltkplatform.hh +++ b/dw/fltkplatform.hh @@ -43,6 +43,7 @@ public: virtual void prepareCreateFltkWidget (); virtual void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); + virtual void removeFltkWidget (::fltk::Widget *widget); virtual void allocateFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); virtual void drawFltkWidget (::fltk::Widget *widget, core::Rectangle *area); diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -77,6 +77,21 @@ FltkResource::~FltkResource () FltkResource::~FltkResource () { platform->detachResource (this); + for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); + it.hasNext(); ) { + ViewAndWidget *viewAndWidget = it.getNext (); + + if (viewAndWidget->widget) { + if (viewAndWidget->view) { + viewAndWidget->view->removeFltkWidget(viewAndWidget->widget); + } + if (viewAndWidget->widget->parent()) { + viewAndWidget->widget->parent()->remove(viewAndWidget->widget); + } + delete viewAndWidget->widget; + } + + } delete viewsAndWidgets; if(style) style->unref (); diff --git a/dw/fltkviewbase.cc b/dw/fltkviewbase.cc --- a/dw/fltkviewbase.cc +++ b/dw/fltkviewbase.cc @@ -364,6 +364,19 @@ void FltkWidgetView::addFltkWidget (::fl allocateFltkWidget (widget, allocation); } +void FltkWidgetView::removeFltkWidget (::fltk::Widget *widget) +{ + for(Iterator <TypedPointer < ::fltk::Widget> > it + = canvasWidgets->iterator (); + it.hasNext (); ) { + TypedPointer < ::fltk::Widget> *wp = it.getNext(); + + if (widget == wp->getTypedValue ()) { + canvasWidgets->remove(wp); + } + } +} + void FltkWidgetView::allocateFltkWidget (::fltk::Widget *widget, core::Allocation *allocation) { diff --git a/dw/fltkviewbase.hh b/dw/fltkviewbase.hh --- a/dw/fltkviewbase.hh +++ b/dw/fltkviewbase.hh @@ -83,6 +83,7 @@ public: bool usesFltkWidgets (); void prepareCreateFltkWidget (); void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); + void removeFltkWidget (::fltk::Widget *widget); void allocateFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); void drawFltkWidget (::fltk::Widget *widget, core::Rectangle *area);

1 0

Re: Accept-Encoding: gzip
by jcid＠dillo.org Nov. 14, 2007

Nov. 14, 2007

Hi, [gzip decoder] On Mon, Nov 12, 2007 at 02:52:43PM +0000, place wrote: > Hmm, how unfortunate. It had been working just fine for me, though I took it > out yesterday so that I could have a clean tree... I wonder whether > we're constantly going to see timing problems or something, where the > people with fast machines write things that work on fast machines and > the person with a slow machine will write things that work on a slow machine. Good news! Committed. I found a workaround for the segfault and decided to commit to allow further investigation and polishing from in-CVS code. It doesn't look like a race condition, but a problem of handling redirections and the null_decoder in cache.c (redirections is a somewhat ad-hoc code, not well designed yet). Attached goes a page to reproduce the segfault (without the patch in CVS). For instance: 1- save the attached page in /tmp 2- dillo-fltk /tmp 3- click on the page 4- go back 5- go forward (segfault) // you may need to repeat this 4 and 5 BTW, the workaround is mainly: - dStr_append_l(entry->Data, buf, (int)buf_size); + /* Assert we have a Decoder. + * BUG: this is a workaround, more study and a proper design + * for handling redirects is required */ + if (entry->Decoder != NULL) { + decodedBuf = a_Decode_process(entry->Decoder, buf, buf_size); + dStr_append_l(entry->Data, decodedBuf->str, decodedBuf->len); + dStr_free(decodedBuf, 1); + } else { + dStr_append_l(entry->Data, buf, buf_size); + } With regard to: // Doesn't work. I could make TotalSize into something like BytesRemaining, // seeing whether it goes precisely to 0. I'd prefer TransferSize (this is the whole http transfer size minus the Header length --i.e. Content-Length). With regard to the iconv decoder: Please note that we may still need the original data, to be able to save verbatim. This is, if the original page is encoded in latin2 (with a <meta http-equiv charset line in the source), and it is saved translated to UTF-8 we have two problems (the misleading "meta" and that the user will get a page that's not a verbatim copy of the original). One way to solve this is to re-encode into the original charset at save time. This is not 8bit clean but could work most of the time. Another way is to keep a copy of the verbatim data. In this case we're 8bit clean and it would only take more memory when the original is not UTF-8. I *feel* 8bit clean is the correct path, and here there're lots of ways to optimize. For instance, with UTF-8 pages we don't need an extra buffer. For pages that need one, we can deallocate the UTF-8 encoded one when leaving the page (and re-create it if the page is visited again). This is just some food for thought. -- Cheers Jorge.-

3 5

Re: patch: TransferSize
by jcid＠dillo.org Nov. 14, 2007

Nov. 14, 2007

On Tue, Nov 13, 2007 at 09:07:48PM +0000, place wrote: > > I'd prefer TransferSize (this is the whole http transfer size minus the > > Header length --i.e. Content-Length). > Committed with some minor changes: s/TotalSize/ExpectedSize/g e.g.: + int ExpectedSize; /* Goal size of the HTTP transfer (0 if unknown)*/ + int TransferSize; /* Actual length of the HTTP transfer */ I also decided to remove this line: entry->Flags |= CA_GotData; entry->Flags &= ~CA_Stopped; /* it may catch up! */ - entry->TotalSize = entry->Data->len; if (entry->Decoder) { a_Decode_free(entry->Decoder); ... and now the transfer-size mismatch messages, shed more light on the redirection bug! For instance if you take the same "mm1.html" test case, going back and forward, it's clear that the image is being reloaded, but the HTTP header is not re-parsed. This is a problem with the cache_null_client. I haven't investigated it further, but this hint should be enough. -- Cheers Jorge.-

1 0

forms related segfault
by Johannes.Hofmann＠gmx.de Nov. 13, 2007

Nov. 13, 2007

Hello, when I go to e.g. www.google.com enter a search, click submit and then when the result page shows up hit Enter, I always get a segfault: Program received signal SIGSEGV, Segmentation fault. loutsignal::Emitter::emitVoid (this=0x28661208, signalNo=0, argc=2, argv=0xbfbff2dc) at container.hh:93 93 inline Iterator iterator() { Iterator it(createIterator()); return it; } (gdb) bt #0 loutsignal::Emitter::emitVoid (this=0x28661208, signalNo=0, argc=2, argv=0xbfbff2dc) at container.hh:93 #1 0x08084367 in dw::core::ui::Resource::ActivateEmitter::emitActivate ( this=0x28661208, resource=0x28661200) at ui.cc:109 #2 0x080909d3 in fltk::Widget::do_callback (this=0x28661280) at Widget.h:125 #3 0x080ab6d1 in fltk::Input::handle_key (this=0x28661280) at Input.cxx:1267 #4 0x080abe42 in fltk::Input::handle (this=0x28661280, event=8, r=@0xbfbff414) at Input.cxx:1469 #5 0x080ac66b in fltk::Input::handle (this=0x28661280, event=8) at Input.cxx:1384 #6 0x080dfac2 in fltk::Widget::send (this=0x28661280, event=8) at Widget.cxx:805 #7 0x080be6dd in fltk::handle (event=8, window=0x28540000) at run.cxx:1287 #8 0x080c4bf4 in fltk::handle () at run.cxx:1947 #9 0x080c4c38 in do_queued_events () at run.cxx:396 #10 0x080c4f52 in fl_wait (time_to_wait=1.00000002e+20) at run.cxx:456 #11 0x080c4fed in fltk::wait (time_to_wait=1.00000002e+20) at run.cxx:457 #12 0x080c5104 in fltk::run () at run.cxx:393 #13 0x0804e714 in main (argc=Error accessing memory address 0x0: Bad address. ) at dillo.cc:107 I think it has to do with the input form of the previous page somehow still being activ. Jorge, do you know what happens to the fltk widgets like buttons or inputs, when a new page gets loaded? Are the deleted, and if so where in the code does this happen? Cheers, Johannes

3 12

Re: is the visited_color pref somehow deprecated?
by jcid＠dillo.org Nov. 13, 2007

Nov. 13, 2007

On Sun, Nov 11, 2007 at 05:04:59PM +0000, place wrote: > dillorc2 doesn't list it, but if you have force_my_colors set and you > don't know to add visited_color to the rc, you get stuck with purple, > which is not optimal on a black background... No it's not deprecated. I tried to add the working options in dillorc2 only. Patch is welcomed! :-) -- Cheers Jorge.-

2 2

Re: image loading from menus
by jcid＠dillo.org Nov. 12, 2007

Nov. 12, 2007

On Sun, Nov 11, 2007 at 01:05:30AM +0000, place wrote: > > > The right-click menus. I am giving them the linkblock ptr via > > > a_UIcmd_*_popup so that they can call Html_load_images > > > (well, a_Html_load_images now, by way of a_UIcmd_load_images -- > > > if that's the right way to route it.) > > > > After some study and a reimplentation of plain.cc with classes > > (getting rid of its linkblock and hooking a callback for memory > > handling), it looks simpler and better. It looks like the same > > may be done with html.cc. > > Has the new html.cc made my problem go away? So far as I can tell, > things will be the same: a_UIcmd_page_popup() will need a DilloHtml*, > so uicmd.hh will include html.hh. When IO/http.c includes uicmd.hh, > it will see #include "dw/core.hh" and #include "lout/misc.hh" in html.hh, > and it will not compile. You can use a void* inside the bw structure (as with render_layout). It may end being a list in the future. For instance, if we ever implement frames, this list could contain the DilloHtml objects for each component URL, of a framed page. The design is not clear yet, but this is one way to implement it. -- Cheers Jorge.-

2 2

Re: patch: save filename/dir
by jcid＠dillo.org Nov. 11, 2007

Nov. 11, 2007

On Sat, Nov 10, 2007 at 11:09:26PM +0000, place wrote: > When I went to save a file, the suggested filename still had the > anchor suffix. Saw from my old tree that URL_PATH() was what I wanted. > Changed that. > > Since I was right there already and there was a comment about a > save dir pref, I added one to prefs.*. > Committed! -- Cheers Jorge.-

1 0

[patch] fix bug in new scrolling code
by Johannes.Hofmann＠gmx.de Nov. 10, 2007

Nov. 10, 2007

Hi, when scrolling using the down arrow key I saw occasionally screen corruptions. The patch below fixes this for me. BTW. the following little program helped me debugging this and might be useful for others with too fast machines. It hogs the CPU, so dillo redraws are much slower and easier to follow: int main(int argc, char **argv) { for (;;); } Cheers, Johannes diff --git a/dw/fltkviewport.cc b/dw/fltkviewport.cc --- a/dw/fltkviewport.cc +++ b/dw/fltkviewport.cc @@ -311,8 +311,11 @@ void FltkViewport::scroll (int dx, int d { int hdiff = vscrollbar->visible () ? SCROLLBAR_THICKNESS : 0; int vdiff = hscrollbar->visible () ? SCROLLBAR_THICKNESS : 0; - int oldScrollX = scrollX; - int oldScrollY = scrollY; + /* multiple calls to scroll can happen before a redraw occurs. + * scrollDX / scrollDY can therefore be non-zero here. + */ + int oldScrollX = scrollX - scrollDX; + int oldScrollY = scrollY - scrollDY; if (dx == 0 && dy == 0) { return;

2 1

[patch] enable partial image drawing
by Johannes.Hofmann＠gmx.de Nov. 9, 2007

Nov. 9, 2007

Hi, here is a first patch to reduce redrawing in dillo-fltk. It just enables code that already existed but was commented out and allows drawing of parts of an image only. I added some additional checks. With these checks I did not see any negative effects. Cheers, Johannes diff --git a/dw/fltkimgbuf.cc b/dw/fltkimgbuf.cc --- a/dw/fltkimgbuf.cc +++ b/dw/fltkimgbuf.cc @@ -334,7 +334,19 @@ void FltkImgbuf::draw (::fltk::Widget *t // xRoot, x, yRoot, y, width, height, this->width, this->height); //{ -#if 0 +#if 1 + if (x > this->width || y > this->height) { + return; + } + + if (x + width > this->width) { + width = this->width - x; + } + + if (y + height > this->height) { + height = this->height - y; + } + // almost OK for rows. For some unknown reason it trims the bottom and // rightmost parts when scrolling. ::fltk::Rectangle rect (xRoot + x, yRoot + y, width, height);

2 1