- Dillo-dev - lists.mailman3.com

Re: Colspan and table percentages

by jcid＠dillo.org

Hi, On Wed, Dec 12, 2007 at 06:09:17PM -0300, Jorge Arellano Cid wrote: > On Tue, Dec 11, 2007 at 01:04:02PM +0000, Jeremy Henty wrote: > > On Mon, Dec 10, 2007 at 06:22:36PM +0100, Johannes Hofmann wrote: > > > > > just noticed that http://www.dillo.org/MList.html still looks weird > > > with dillo-fltk: The main text is rendered just one word wide here. > > > > I see this quite a lot. I've attached a simple example below. It > > depends on the table cell having colspan="2". Not only does each word > > appear on a line by itself but no horizontal scrollbar appears even > > though one word extends past the right of the page. > > OK, a patch was committed to CVS. It fixes this issue but some > glitches remain. > > With http://www.dillo.org/MList.html, the "Bug Track Engine" > cell is too narrow. The main problem is that, AFAIS, new data > structures&algorithm is necessary to handle the special case > where a colsapanned cell has a larger width than the spanned > cells. > Some workarounds can be coded but they corrupt the meaning of > the current data structures. I'll try to find a solution... OK, a new patch with improved algorithms, using the same data structures, was committed. This time rendering is much better. Please test and send feedback, URLs. It looks like the "gross work" is done. Some polishing remains, but that's subject to another patch. 8-) -- Cheers Jorge.-

Dec. 15, 2007

[PATCH] implements dw::fltk::ui::FltkMultiLineTextResource

by onepoint＠starurchin.org

I promised you all something more substantial than whitespace and comment fixes next, didn't I? This patch implements the dw::fltk::ui::FltkMultiLineTextResource class, using ::fltk:LMultiLineInput to create the widget. It also adds a FltkMultiLineTextResource to the dw-ui-test example. This isn't complete yet; the number of rows and columns are hardwired. Comments are welcome. Regards, Jeremy Henty diff -pru -- dw2-ref/dw/fltkplatform.cc dw2-cur/dw/fltkplatform.cc --- dw2-ref/dw/fltkplatform.cc 2007-12-06 17:29:59.000000000 +0000 +++ dw2-cur/dw/fltkplatform.cc 2007-12-15 01:41:02.000000000 +0000 @@ -144,8 +144,7 @@ FltkPlatform::FltkResourceFactory::creat core::ui::MultiLineTextResource * FltkPlatform::FltkResourceFactory::createMultiLineTextResource () { - /** \bug Not implemented. */ - return NULL; + return new ui::FltkMultiLineTextResource (platform); } core::ui::CheckButtonResource * diff -pru -- dw2-ref/dw/fltkui.cc dw2-cur/dw/fltkui.cc --- dw2-ref/dw/fltkui.cc 2007-12-06 17:29:59.000000000 +0000 +++ dw2-cur/dw/fltkui.cc 2007-12-15 02:34:13.000000000 +0000 @@ -30,6 +30,7 @@ #include <fltk/Group.h> #include <fltk/Input.h> #include <fltk/SecretInput.h> +#include <fltk/MultiLineInput.h> #include <fltk/RadioButton.h> #include <fltk/CheckButton.h> #include <fltk/Choice.h> @@ -527,6 +528,107 @@ void FltkEntryResource::setEditable (boo // ---------------------------------------------------------------------- +FltkMultiLineTextResource::FltkMultiLineTextResource (FltkPlatform *platform): + FltkSpecificResource <dw::core::ui::MultiLineTextResource> (platform) +{ + initText = NULL; + editable = false; + + init (platform); +} + +FltkMultiLineTextResource::~FltkMultiLineTextResource () +{ + if (initText) + delete initText; +} + +::fltk::Widget *FltkMultiLineTextResource::createNewWidget (core::Allocation + *allocation) +{ + ::fltk::MultiLineInput *input = + new ::fltk::MultiLineInput (allocation->x, allocation->y, + allocation->width, + allocation->ascent + allocation->descent); + input->callback (widgetCallback, this); + input->when (::fltk::WHEN_ENTER_KEY_ALWAYS); + + if (viewsAndWidgets->isEmpty ()) { + // First widget created, attach the set text. + if (initText) + input->value (initText); + } else + input->value + (((::fltk::MultiLineInput*)viewsAndWidgets->getFirst()->widget)->value ()); + + return input; +} + +void FltkMultiLineTextResource::sizeRequest (core::Requisition *requisition) +{ + if (style) { + FltkFont *font = (FltkFont*)style->font; + requisition->width = + (int)::fltk::getwidth ("M", 1) * 20 + + 2 * RELIEF_X_THICKNESS; + requisition->ascent = RELIEF_Y_THICKNESS; + requisition->descent = + (font->ascent + font->descent) * 10 + RELIEF_Y_THICKNESS; + } else { + requisition->width = 1; + requisition->ascent = 1; + requisition->descent = 0; + } +} + +void FltkMultiLineTextResource::widgetCallback (::fltk::Widget *widget, + void *data) +{ + /* The (::fltk::event_key() == ::fltk::ReturnKey) test + * is necessary because WHEN_ENTER_KEY also includes + * other events we're not interested in. For instance pressing + * The Back or Forward, buttons, or the first click on a rendered + * page. BUG: this must be investigated and reported to FLTK2 team + */ + printf ("when = %d\n", widget->when ()); + if ((widget->when () & ::fltk::WHEN_ENTER_KEY_ALWAYS) && + (::fltk::event_key() == ::fltk::ReturnKey)) + ((FltkMultiLineTextResource*)data)->emitActivate (); +} + +const char *FltkMultiLineTextResource::getText () +{ + if (viewsAndWidgets->isEmpty ()) + return initText; + else + return ((::fltk::MultiLineInput*)viewsAndWidgets->getFirst()->widget)->value (); +} + +void FltkMultiLineTextResource::setText (const char *text) +{ + if (initText) + delete initText; + initText = strdup (text); + + for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); + it.hasNext(); ) { + ViewAndWidget *viewAndWidget = it.getNext (); + ((::fltk::MultiLineInput*)viewAndWidget->widget)->value (initText); + } +} + +bool FltkMultiLineTextResource::isEditable () +{ + return editable; +} + +void FltkMultiLineTextResource::setEditable (bool editable) +{ + this->editable = editable; +} + +// ---------------------------------------------------------------------- + template <class I> FltkToggleButtonResource<I>::FltkToggleButtonResource (FltkPlatform *platform, bool activated): diff -pru -- dw2-ref/dw/fltkui.hh dw2-cur/dw/fltkui.hh --- dw2-ref/dw/fltkui.hh 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/dw/fltkui.hh 2007-12-15 01:28:53.000000000 +0000 @@ -313,6 +313,31 @@ public: }; +class FltkMultiLineTextResource: + public FltkSpecificResource <dw::core::ui::MultiLineTextResource> +{ +private: + const char *initText; + bool editable; + + static void widgetCallback (::fltk::Widget *widget, void *data); + +protected: + ::fltk::Widget *createNewWidget (core::Allocation *allocation); + +public: + FltkMultiLineTextResource (FltkPlatform *platform); + ~FltkMultiLineTextResource (); + + void sizeRequest (core::Requisition *requisition); + + const char *getText (); + void setText (const char *text); + bool isEditable (); + void setEditable (bool editable); +}; + + template <class I> class FltkToggleButtonResource: public FltkSpecificResource <I> { diff -pru -- dw2-ref/test/dw_ui_test.cc dw2-cur/test/dw_ui_test.cc --- dw2-ref/test/dw_ui_test.cc 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/test/dw_ui_test.cc 2007-12-15 02:35:22.000000000 +0000 @@ -44,10 +44,10 @@ int main(int argc, char **argv) FltkPlatform *platform = new FltkPlatform (); Layout *layout = new Layout (platform); - ::fltk::Window *window = new ::fltk::Window(300, 300, "Dw UI Test"); + ::fltk::Window *window = new ::fltk::Window(400, 400, "Dw UI Test"); window->begin(); - FltkViewport *viewport = new FltkViewport (0, 0, 300, 300); + FltkViewport *viewport = new FltkViewport (0, 0, 400, 400); layout->attachView (viewport); StyleAttrs styleAttrs; @@ -83,6 +83,8 @@ int main(int argc, char **argv) entryres1->setText ("Hi!"); EntryResource *entryres2 = layout->getResourceFactory()->createEntryResource (10, true); + MultiLineTextResource *textres = + layout->getResourceFactory()->createMultiLineTextResource (); RadioButtonResource *radiores1 = layout->getResourceFactory()->createRadioButtonResource (NULL, false); RadioButtonResource *radiores2 = @@ -115,6 +117,7 @@ int main(int argc, char **argv) form::Form *form = new form::Form(); form->addTextResource ("val1", entryres1); form->addTextResource ("val2", entryres2); + form->addTextResource ("text", textres); const char *radiovalues[] = { "radio1", "radio2", NULL }; form->addRadioButtonResource ("val3", radiores1, radiovalues); form->addCheckButtonResource ("check", checkres); @@ -151,6 +154,19 @@ int main(int argc, char **argv) table->addCell (input2, 1, 1); table->addRow (cellStyle); + + Textblock *label = new Textblock(false); + label->setStyle (cellStyle); + table->addCell (label, 1, 1); + label->addText (strdup ("text = "), cellStyle); + label->flush (); + + Embed *text = new Embed (textres); + text->setStyle (cellStyle); + table->addCell (text, 1, 1); + + table->addRow (cellStyle); + Textblock *radiolabel1 = new Textblock(false); radiolabel1->setStyle (cellStyle); table->addCell (radiolabel1, 2, 1);

Dec. 15, 2007

[PATCH] comment fix in dw2/dw/iterator.hh

by jcid＠dillo.org

On Thu, Dec 13, 2007 at 02:49:51PM +0000, Jeremy Henty wrote: > > The comment near the top of dw2/dw/iterator.hh refers to two methods > dw::core::Iterator::pointsOnRealContent and dw::core::hasAnyContents, > neither of which exist. (In fact there's no case-insensitive match > for pointsOn, OnReal, RealContent, hasAny or AnyContents anywhere in > the source.) The only similarly-named method I can find is > dw::core::Widget::hasContents so I've changed the comment to mention > that. Committed. > Or maybe the comment should be rewritten or deleted? Good question. When understanding of the code is complete, commenting is easier. :-) -- Cheers Jorge.-

Dec. 14, 2007

should new windows inherit panel size and fullscreen mode?

by Johannes.Hofmann＠gmx.de

Ok, here is a patch that opens new windows with the same style as the current one. Cheers, Johannes

Dec. 13, 2007

[PATCH] comment fix in dw2/dw/iterator.hh

by onepoint＠starurchin.org

The comment near the top of dw2/dw/iterator.hh refers to two methods dw::core::Iterator::pointsOnRealContent and dw::core::hasAnyContents, neither of which exist. (In fact there's no case-insensitive match for pointsOn, OnReal, RealContent, hasAny or AnyContents anywhere in the source.) The only similarly-named method I can find is dw::core::Widget::hasContents so I've changed the comment to mention that. Or maybe the comment should be rewritten or deleted? The next patch will be non-cosmetic, I just wanted to get some cleanups out of my TO-DO. Regards, Jeremy Henty diff -pru -- dw2-ref/dw/iterator.hh dw2-cur/dw/iterator.hh --- dw2-ref/dw/iterator.hh 2007-12-11 11:30:07.000000000 +0000 +++ dw2-cur/dw/iterator.hh 2007-12-13 14:26:43.000000000 +0000 @@ -11,9 +11,8 @@ namespace core { /** * \brief Iterators are used to iterate through the contents of a widget. * - * When using iterators, you should care about the results of - * dw::core::Iterator::pointsOnRealContent and especially - * dw::core::hasAnyContents. + * When using iterators, you should care about the results of + * dw::core::Widget::hasContents. * * \sa dw::core::Widget::iterator */

Dec. 13, 2007

Re: Colspan and table percentages

by jcid＠dillo.org

On Tue, Dec 11, 2007 at 01:04:02PM +0000, Jeremy Henty wrote: > On Mon, Dec 10, 2007 at 06:22:36PM +0100, Johannes Hofmann wrote: > > > just noticed that http://www.dillo.org/MList.html still looks weird > > with dillo-fltk: The main text is rendered just one word wide here. > > I see this quite a lot. I've attached a simple example below. It > depends on the table cell having colspan="2". Not only does each word > appear on a line by itself but no horizontal scrollbar appears even > though one word extends past the right of the page. OK, a patch was committed to CVS. It fixes this issue but some glitches remain. With http://www.dillo.org/MList.html, the "Bug Track Engine" cell is too narrow. The main problem is that, AFAIS, new data structures&algorithm is necessary to handle the special case where a colsapanned cell has a larger width than the spanned cells. Some workarounds can be coded but they corrupt the meaning of the current data structures. I'll try to find a solution... Please test and post the URLs where you found the problem. -- Cheers Jorge.-

Dec. 12, 2007

[PATCH] DW: indentation in dw/selection.cc

by jcid＠dillo.org

On Tue, Dec 11, 2007 at 09:16:19PM +0000, Jeremy Henty wrote: > > This is purely cosmetic: my changes to dw/selection.cc messed up the > indentation so this patch fixes it. Committed. -- Cheers Jorge.-

Dec. 12, 2007

[PATCH] DW: indentation in dw/selection.cc

by onepoint＠starurchin.org

This is purely cosmetic: my changes to dw/selection.cc messed up the indentation so this patch fixes it. Incidentally, does anyone have a version of the emacs C/C++ modes that is consistent with the Dillo source? I've set: c-basic offset: 3 indent-tabs-mode: nil but it still pointlessly reformats the source. In particular it indents the contents of namespace { ... }. Regards, Jeremy Henty diff -pru -- dw2-ref/dw/selection.cc dw2-cur/dw/selection.cc --- dw2-ref/dw/selection.cc 2007-12-11 11:30:07.000000000 +0000 +++ dw2-cur/dw/selection.cc 2007-12-11 20:54:10.000000000 +0000 @@ -123,19 +123,19 @@ bool SelectionState::buttonPress (Iterat linkState = LINK_PRESSED; linkButton = event->button; DeepIterator *newLink = new DeepIterator (it); - if (newLink->isEmpty ()) { - delete newLink; - resetLink (); - } else { - link = newLink; - // It may be that the user has pressed on something activatable - // (linkNo != -1), but there is no contents, e.g. with images - // without ALTernative text. - if (link) { - linkChar = correctCharPos (link, charPos); - linkNumber = linkNo; - } - } + if (newLink->isEmpty ()) { + delete newLink; + resetLink (); + } else { + link = newLink; + // It may be that the user has pressed on something activatable + // (linkNo != -1), but there is no contents, e.g. with images + // without ALTernative text. + if (link) { + linkChar = correctCharPos (link, charPos); + linkNumber = linkNo; + } + } // We do not return the value of the signal method, // but we do actually process this event. ret = true; @@ -148,15 +148,15 @@ bool SelectionState::buttonPress (Iterat selectionState = SELECTING; DeepIterator *newFrom = new DeepIterator (it); - if (newFrom->isEmpty ()) { - delete newFrom; - resetSelection (); - } else { - from = newFrom; - fromChar = correctCharPos (from, charPos); - to = from->cloneDeepIterator (); - toChar = correctCharPos (to, charPos); - } + if (newFrom->isEmpty ()) { + delete newFrom; + resetSelection (); + } else { + from = newFrom; + fromChar = correctCharPos (from, charPos); + to = from->cloneDeepIterator (); + toChar = correctCharPos (to, charPos); + } ret = true; } else { if (event && event->button == 3) {

Dec. 11, 2007

should new windows inherit panel size and fullscreen mode?

by onepoint＠starurchin.org

On Tue, Dec 11, 2007 at 04:56:46PM +0100, Johannes Hofmann wrote: > I would like that new browser windows inherit the panel size and > fullscreen mode from the current window. What do others think? +1 Jeremy Henty

Dec. 11, 2007

should new windows inherit panel size and fullscreen mode?

by jcid＠dillo.org

On Tue, Dec 11, 2007 at 04:56:46PM +0100, Johannes Hofmann wrote: > Hello, > > I would like that new browser windows inherit the panel size and > fullscreen mode from the current window. What do others think? AFAIS, that's the desired behaviour. -- Cheers Jorge.-

Dec. 11, 2007

should new windows inherit panel size and fullscreen mode?

by Johannes.Hofmann＠gmx.de

Hello, I would like that new browser windows inherit the panel size and fullscreen mode from the current window. What do others think? Cheers, Johannes

Dec. 11, 2007

Re: Colspan and table percentages

by Johannes.Hofmann＠gmx.de

On Tue, Dec 11, 2007 at 10:16:01AM -0300, Jorge Arellano Cid wrote: > On Mon, Dec 10, 2007 at 06:22:36PM +0100, Johannes Hofmann wrote: > > Hi Jorge, > > > > just noticed that http://www.dillo.org/MList.html still looks weird > > with dillo-fltk: The main text is rendered just one word wide here. > > Yes, that's a special colspan case. > > Please test now! Works fine now. Thanks, Johannes > > -- > Cheers > Jorge.- > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 11, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

On Tue, Dec 11, 2007 at 12:06:37PM +0000, Jeremy Henty wrote: > On Mon, Dec 10, 2007 at 05:44:08PM +0100, Johannes Hofmann wrote: > > > Just one thing. I would rather have tested the return value of > > getTopIterator() for NULL. > > Heh, that *was* my original implementation of isEmpty() . Then I > noticed the hasContents member so I used that since it shortened the > code. I believe hasContents could be eliminated since it is true when > and only when the stack is non-empty, though I haven't exhaustively > checked this. > > > I think the code is generally a bit optimistic here by assuming > > non-NULL return value from getTopIterator(). Even if this is now > > always the case with your fix, it may change again in the future. > > Yes it may. But I think much of the code assumes this, so if this > changes in the future then a lot of code will need to be reviewed, > preferably by someone who understands the design. > > > What do you think, if we add checks for NULL at those three points > > in the code where getTopIterator() is called, do we still need to > > check with isEmpty() to call resetLink() / resetSelection() in > > buttonPress()? > > Yes, otherwise you get a crash in DeepIterator::compareTo, which also > assumes that the stack is non-empty. (In fact the comments suggest > that it assumes the tops of the stacks of the two DeepIterator objects > point to the same widget.) When I found that crash I decided to give > up on rewriting lots of code to handle empty DeepIterators and instead > simply prevent empty DeepIterators appearing in Selection objects. > > Even if other code ought to handle empty DeepIterators better, I think > it's reasonable to argue that Selection objects should never use them. > After all, what could it possible mean for a selection to begin or end > at an empty DeepIterator? So I think my bugfix makes Selections do > the right thing, even if other code should be checking > getTopIterator() as you suggest. Yes, I fully agree with you. Selection is fixed. We may need to come back to DeepIterator later. Regards, Johannes > > Regards, > > Jeremy Henty > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 11, 2007

Re: Colspan and table percentages

by onepoint＠starurchin.org

On Tue, Dec 11, 2007 at 01:04:02PM +0000, Jeremy Henty (ie. me) wrote: > I've attached a simple example below. It depends on the table cell > having colspan="2". Not only does each word appear on a line by > itself OK, this is now fixed. > ... but no horizontal scrollbar appears even though one word extends > past the right of the page. This issue remains. Regards, Jeremy Henty <?xml version="1.0" encoding="UTF-8"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html> <head> <title> Dillo: display and layout strangeness in a table cell with colspan="2" </title> </head> <body> <table> <tr> <td colspan="2"> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbb cccccc dddddddd eeeeee </td> </tr> </table> </body> </html> _______________________________________________ Dillo-dev mailing list Dillo-dev(a)dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 11, 2007

[patch] fix close button height in page source window

by jcid＠dillo.org

On Mon, Dec 10, 2007 at 06:15:56PM +0100, Johannes Hofmann wrote: > Hi, > > the height of the close button of the source code window changes > with the size of the window. The patch below fixes its size. Committed. > PS: Scrolling in the page source window is dead slow here. But > that's definately a fltk issue... Yes, and we still have the resort of showing the source inside a new dillo browser window (this would add find text, but lose the nice line numbers), with the gain of faster scrolling. Now, if we delegate viewsource to a dpi, it would be simple to add the line numbers. Just some thoughts. -- Cheers Jorge.-

Dec. 11, 2007

Re: Colspan and table percentages

by jcid＠dillo.org

On Mon, Dec 10, 2007 at 06:22:36PM +0100, Johannes Hofmann wrote: > Hi Jorge, > > just noticed that http://www.dillo.org/MList.html still looks weird > with dillo-fltk: The main text is rendered just one word wide here. Yes, that's a special colspan case. Please test now! -- Cheers Jorge.-

Dec. 11, 2007

Re: Colspan and table percentages

by onepoint＠starurchin.org

On Mon, Dec 10, 2007 at 06:22:36PM +0100, Johannes Hofmann wrote: > just noticed that http://www.dillo.org/MList.html still looks weird > with dillo-fltk: The main text is rendered just one word wide here. I see this quite a lot. I've attached a simple example below. It depends on the table cell having colspan="2". Not only does each word appear on a line by itself but no horizontal scrollbar appears even though one word extends past the right of the page. Regards, Jeremy Henty <?xml version="1.0" encoding="UTF-8"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html> <head> <title> Dillo: display and layout strangeness in a table cell with colspan="2" </title> </head> <body> <table> <tr> <td colspan="2"> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbb cccccc dddddddd eeeeee </td> </tr> </table> </body> </html>

Dec. 11, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

On Mon, Dec 10, 2007 at 05:44:08PM +0100, Johannes Hofmann wrote: > Just one thing. I would rather have tested the return value of > getTopIterator() for NULL. Heh, that *was* my original implementation of isEmpty() . Then I noticed the hasContents member so I used that since it shortened the code. I believe hasContents could be eliminated since it is true when and only when the stack is non-empty, though I haven't exhaustively checked this. > I think the code is generally a bit optimistic here by assuming > non-NULL return value from getTopIterator(). Even if this is now > always the case with your fix, it may change again in the future. Yes it may. But I think much of the code assumes this, so if this changes in the future then a lot of code will need to be reviewed, preferably by someone who understands the design. > What do you think, if we add checks for NULL at those three points > in the code where getTopIterator() is called, do we still need to > check with isEmpty() to call resetLink() / resetSelection() in > buttonPress()? Yes, otherwise you get a crash in DeepIterator::compareTo, which also assumes that the stack is non-empty. (In fact the comments suggest that it assumes the tops of the stacks of the two DeepIterator objects point to the same widget.) When I found that crash I decided to give up on rewriting lots of code to handle empty DeepIterators and instead simply prevent empty DeepIterators appearing in Selection objects. Even if other code ought to handle empty DeepIterators better, I think it's reasonable to argue that Selection objects should never use them. After all, what could it possible mean for a selection to begin or end at an empty DeepIterator? So I think my bugfix makes Selections do the right thing, even if other code should be checking getTopIterator() as you suggest. Regards, Jeremy Henty

Dec. 11, 2007

Re: Colspan and table percentages

by Johannes.Hofmann＠gmx.de

Hi Jorge, just noticed that http://www.dillo.org/MList.html still looks weird with dillo-fltk: The main text is rendered just one word wide here. Cheers, Johannes On Sun, Dec 09, 2007 at 05:58:04PM -0300, Jorge Arellano Cid wrote: > On Sun, Dec 09, 2007 at 02:07:48PM -0300, Jorge Arellano Cid wrote: > > Hi! > > > > After a lot of work, the patch for better table rendering > > (regarding colspan and percentages) was good enough to be > > committed. Please enjoy and send feedback (or patches) ;-). > > > > - Implemented a weighted apportionment algorithm for table colspan. > > - Implemented percentage widths in tables (better rendering!). > > - Fixed an initialization bug in hruler. > > + Minor fix to table-wide percentage apportion. > > Now about:splash renders OK. > > -- > Cheers > Jorge.- > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 10, 2007

[patch] fix close button height in page source window

by Johannes.Hofmann＠gmx.de

Hi, the height of the close button of the source code window changes with the size of the window. The patch below fixes its size. Cheers, Johannes PS: Scrolling in the page source window is dead slow here. But that's definately a fltk issue... diff -r 043b53a25dc3 src/dialog.cc --- a/src/dialog.cc Mon Dec 10 15:42:03 2007 +0100 +++ b/src/dialog.cc Mon Dec 10 18:12:15 2007 +0100 @@ -123,7 +123,7 @@ void a_Dialog_text_window(const char *tx ReturnButton *b = new ReturnButton (0, wh-bh, ww, bh, "Close"); b->callback(window_close_cb, window); - window->resizable(window); + window->resizable(td); window->end(); window->show(); }

Dec. 10, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

Hi Jeremy, On Fri, Dec 07, 2007 at 09:02:39PM +0000, Jeremy Henty wrote: > On Fri, Dec 07, 2007 at 06:57:26PM +0000, Jeremy Henty (ie. me) wrote: > > > * [Johannes Hofmann's] fix for the DeepIterator search methods is > > the correct fix for the original problem. > > > > * The correct fix for *this* problem is for SelectionState objects > > to check any DeepIterator objects they create and reset the > > link/selection if the new DeepIterator object is empty. > > OK, I think I have this working. Please test! Johannes' patch for > DeepIterator::{searchDownward,searchSideward}() is required. His > patch for lout/container.hh is not (I think) required, but I feel it > should go in anyway; it would have been nice to have been faced with > an obvious NULL rather than memory corruption. Here's the patch to > SelectionState::buttonPress() (which also adds > DeepIterator::isEmpty() for convenience). > > Comments are welcome! Yes great. Your patch fixes the problem here! Just one thing. I would rather have tested the return value of getTopIterator() for NULL. I think the code is generally a bit optimistic here by assuming non-NULL return value from getTopIterator(). Even if this is now always the case with your fix, it may change again in the future. What do you think, if we add checks for NULL at those three points in the code where getTopIterator() is called, do we still need to check with isEmpty() to call resetLink() / resetSelection() in buttonPress()? Regards, Johannes > > Jeremy Henty > > > Only in dw2-cur: .in > 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 > Only in dw2-cur/dw: findtext.o > diff -pru -- dw2-ref/dw/iterator.cc dw2-cur/dw/iterator.cc > --- dw2-ref/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 > +++ dw2-cur/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 > @@ -591,6 +591,10 @@ DeepIterator *DeepIterator::createVarian > return new DeepIterator (it); > } > > +bool DeepIterator::isEmpty () { > + return ! hasContents; > +} > + > /** > * \brief Move iterator forward and store content it. > * > diff -pru -- dw2-ref/dw/iterator.hh dw2-cur/dw/iterator.hh > --- dw2-ref/dw/iterator.hh 2007-10-06 23:03:01.000000000 +0100 > +++ dw2-cur/dw/iterator.hh 2007-12-07 19:07:09.000000000 +0000 > @@ -171,6 +171,8 @@ public: > inline Iterator *getTopIterator () { return stack.getTop(); } > inline Content *getContent () { return &content; } > > + bool isEmpty (); > + > bool next (); > bool prev (); > inline DeepIterator *cloneDeepIterator() { return (DeepIterator*)clone(); } > Only in dw2-cur/dw: iterator.o > Only in dw2-cur/dw: layout.o > diff -pru -- dw2-ref/dw/selection.cc dw2-cur/dw/selection.cc > --- dw2-ref/dw/selection.cc 2007-11-19 15:58:52.000000000 +0000 > +++ dw2-cur/dw/selection.cc 2007-12-07 19:08:36.000000000 +0000 > @@ -122,15 +122,20 @@ bool SelectionState::buttonPress (Iterat > resetLink (); > linkState = LINK_PRESSED; > linkButton = event->button; > - link = new DeepIterator (it); > - > - // It may be that the user has pressed on something activatable > - // (linkNo != -1), but there is no contents, e.g. with images > - // without ALTernative text. > - if (link) { > - linkChar = correctCharPos (link, charPos); > - linkNumber = linkNo; > - } > + DeepIterator *newLink = new DeepIterator (it); > + if (newLink->isEmpty ()) { > + delete newLink; > + resetLink (); > + } else { > + link = newLink; > + // It may be that the user has pressed on something activatable > + // (linkNo != -1), but there is no contents, e.g. with images > + // without ALTernative text. > + if (link) { > + linkChar = correctCharPos (link, charPos); > + linkNumber = linkNo; > + } > + } > // We do not return the value of the signal method, > // but we do actually process this event. > ret = true; > @@ -142,10 +147,16 @@ bool SelectionState::buttonPress (Iterat > resetSelection (); > > selectionState = SELECTING; > - from = new DeepIterator (it); > - fromChar = correctCharPos (from, charPos); > - to = from->cloneDeepIterator (); > - toChar = correctCharPos (to, charPos); > + DeepIterator *newFrom = new DeepIterator (it); > + if (newFrom->isEmpty ()) { > + delete newFrom; > + resetSelection (); > + } else { > + from = newFrom; > + fromChar = correctCharPos (from, charPos); > + to = from->cloneDeepIterator (); > + toChar = correctCharPos (to, charPos); > + } > ret = true; > } else { > if (event && event->button == 3) { > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 10, 2007

Re: Colspan and table percentages

by Johannes.Hofmann＠gmx.de

On Sun, Dec 09, 2007 at 05:58:04PM -0300, Jorge Arellano Cid wrote: > On Sun, Dec 09, 2007 at 02:07:48PM -0300, Jorge Arellano Cid wrote: > > Hi! > > > > After a lot of work, the patch for better table rendering > > (regarding colspan and percentages) was good enough to be > > committed. Please enjoy and send feedback (or patches) ;-). > > > > - Implemented a weighted apportionment algorithm for table colspan. > > - Implemented percentage widths in tables (better rendering!). > > - Fixed an initialization bug in hruler. > > + Minor fix to table-wide percentage apportion. > > Now about:splash renders OK. Excellent, dpi:/bm/ renders fine too! Cheers, Johannes > > -- > Cheers > Jorge.- > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 10, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by jcid＠dillo.org

On Thu, Dec 06, 2007 at 06:12:47PM +0000, Jeremy Henty wrote: > On Thu, Dec 06, 2007 at 05:19:34PM +0100, Johannes Hofmann wrote: > > > [snip] > > fixes the crashes for me. Please test. > > Possibly only a partial fix, I'm afraid. The crash on the page I sent > you has gone, but I have a test file containing just an image, and > when I click to the right of that image dillo segfaults. gdb says: > > Program terminated with signal 11, Segmentation fault. > #0 0x080989c3 in dw::core::SelectionState::correctCharPos (it=0x810dfe8, > charPos=1073741824) at selection.cc:362 > 362 if (top->getContent()->type == Content::TEXT) > (gdb) print top > $1 = (class dw::core::Iterator *) 0x19 > (gdb) print top->getContent() > Cannot access memory at address 0x19 > (gdb) print it > $2 = (class dw::core::DeepIterator *) 0x810dfe8 > > That 0x19 pointer looks like memory corruption, and valgrind confirms > that dillo is accessing freed memory. I'll dig into its output and > see what sense I can make. > > Of course this might be a completely different bug from the one your > patch fixes. Perhaps you could assume that for the moment and commit > anyway? Well, at least that patch fixes the problem with this test case: <html><body> <table width="100%" border="1"> <tr> <td><img src="continued_a.gif" alt="CONTINUED BELOW" height="22" width="59" border="0"></td> </table> </body></html> (clicking to the right of text, inside the table, made dillo exit). and as it really looks like a couple of typos, it was committed! :-) The other patch fixes this test case: <html><body> <img src="continued_a.gif" alt="CONTINUED BELOW" height="22" width="59" border="0"> </body></html> I also don't know whether this is the correct fix for the design, but as it fixes a nasty bug, I preferred to commit. -- Cheers Jorge.-

Dec. 10, 2007

Re: Colspan and table percentages

by jcid＠dillo.org

On Sun, Dec 09, 2007 at 02:07:48PM -0300, Jorge Arellano Cid wrote: > Hi! > > After a lot of work, the patch for better table rendering > (regarding colspan and percentages) was good enough to be > committed. Please enjoy and send feedback (or patches) ;-). > > - Implemented a weighted apportionment algorithm for table colspan. > - Implemented percentage widths in tables (better rendering!). > - Fixed an initialization bug in hruler. + Minor fix to table-wide percentage apportion. Now about:splash renders OK. -- Cheers Jorge.-

Dec. 9, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by jcid＠dillo.org

On Fri, Dec 07, 2007 at 09:02:39PM +0000, Jeremy Henty wrote: > On Fri, Dec 07, 2007 at 06:57:26PM +0000, Jeremy Henty (ie. me) wrote: > > > * [Johannes Hofmann's] fix for the DeepIterator search methods is > > the correct fix for the original problem. > > > > * The correct fix for *this* problem is for SelectionState objects > > to check any DeepIterator objects they create and reset the > > link/selection if the new DeepIterator object is empty. > > OK, I think I have this working. Please test! Johannes' patch for > DeepIterator::{searchDownward,searchSideward}() is required. His > patch for lout/container.hh is not (I think) required, but I feel it > should go in anyway; it would have been nice to have been faced with > an obvious NULL rather than memory corruption. Here's the patch to > SelectionState::buttonPress() (which also adds > DeepIterator::isEmpty() for convenience). > > Comments are welcome! OK, then I'll wait for Johannes comments on this before a commit. You seem to have investigated it to a good depth. If you finally make it into the bottom and are confident enough of how it is done, just let us know to commit. BTW, the last table rendering patch, fixes a problem with horizontal rulers (unsetting the HAS_CONTENTS flags). This may have lead an iterator to traverse uninitialized memory. -- Cheers Jorge.-

Dec. 9, 2007

Colspan and table percentages

by jcid＠dillo.org

Hi! After a lot of work, the patch for better table rendering (regarding colspan and percentages) was good enough to be committed. Please enjoy and send feedback (or patches) ;-). - Implemented a weighted apportionment algorithm for table colspan. - Implemented percentage widths in tables (better rendering!). - Fixed an initialization bug in hruler. If you: #define DBG In table.cc. Verbose debugging output is enabled. I tried to make special and ill cases decision-making similar to firefox, but frankly, some cases are really weird! For instance, take a look here and compare: http://www.dillo.org/test/cs1.html http://www.dillo.org/test/cs7.html // e.g. look at th 20% table Well, the main point is for you to test and to report how it works! -- Cheers Jorge.-

Dec. 9, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

On Fri, Dec 07, 2007 at 06:57:26PM +0000, Jeremy Henty (ie. me) wrote: > * [Johannes Hofmann's] fix for the DeepIterator search methods is > the correct fix for the original problem. > > * The correct fix for *this* problem is for SelectionState objects > to check any DeepIterator objects they create and reset the > link/selection if the new DeepIterator object is empty. OK, I think I have this working. Please test! Johannes' patch for DeepIterator::{searchDownward,searchSideward}() is required. His patch for lout/container.hh is not (I think) required, but I feel it should go in anyway; it would have been nice to have been faced with an obvious NULL rather than memory corruption. Here's the patch to SelectionState::buttonPress() (which also adds DeepIterator::isEmpty() for convenience). Comments are welcome! Jeremy Henty Only in dw2-cur: .in 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 Only in dw2-cur/dw: findtext.o diff -pru -- dw2-ref/dw/iterator.cc dw2-cur/dw/iterator.cc --- dw2-ref/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 +++ dw2-cur/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 @@ -591,6 +591,10 @@ DeepIterator *DeepIterator::createVarian return new DeepIterator (it); } +bool DeepIterator::isEmpty () { + return ! hasContents; +} + /** * \brief Move iterator forward and store content it. * diff -pru -- dw2-ref/dw/iterator.hh dw2-cur/dw/iterator.hh --- dw2-ref/dw/iterator.hh 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/dw/iterator.hh 2007-12-07 19:07:09.000000000 +0000 @@ -171,6 +171,8 @@ public: inline Iterator *getTopIterator () { return stack.getTop(); } inline Content *getContent () { return &content; } + bool isEmpty (); + bool next (); bool prev (); inline DeepIterator *cloneDeepIterator() { return (DeepIterator*)clone(); } Only in dw2-cur/dw: iterator.o Only in dw2-cur/dw: layout.o diff -pru -- dw2-ref/dw/selection.cc dw2-cur/dw/selection.cc --- dw2-ref/dw/selection.cc 2007-11-19 15:58:52.000000000 +0000 +++ dw2-cur/dw/selection.cc 2007-12-07 19:08:36.000000000 +0000 @@ -122,15 +122,20 @@ bool SelectionState::buttonPress (Iterat resetLink (); linkState = LINK_PRESSED; linkButton = event->button; - link = new DeepIterator (it); - - // It may be that the user has pressed on something activatable - // (linkNo != -1), but there is no contents, e.g. with images - // without ALTernative text. - if (link) { - linkChar = correctCharPos (link, charPos); - linkNumber = linkNo; - } + DeepIterator *newLink = new DeepIterator (it); + if (newLink->isEmpty ()) { + delete newLink; + resetLink (); + } else { + link = newLink; + // It may be that the user has pressed on something activatable + // (linkNo != -1), but there is no contents, e.g. with images + // without ALTernative text. + if (link) { + linkChar = correctCharPos (link, charPos); + linkNumber = linkNo; + } + } // We do not return the value of the signal method, // but we do actually process this event. ret = true; @@ -142,10 +147,16 @@ bool SelectionState::buttonPress (Iterat resetSelection (); selectionState = SELECTING; - from = new DeepIterator (it); - fromChar = correctCharPos (from, charPos); - to = from->cloneDeepIterator (); - toChar = correctCharPos (to, charPos); + DeepIterator *newFrom = new DeepIterator (it); + if (newFrom->isEmpty ()) { + delete newFrom; + resetSelection (); + } else { + from = newFrom; + fromChar = correctCharPos (from, charPos); + to = from->cloneDeepIterator (); + toChar = correctCharPos (to, charPos); + } ret = true; } else { if (event && event->button == 3) {

Dec. 7, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

On Fri, Dec 07, 2007 at 08:45:00AM +0100, Johannes Hofmann wrote: > The stack seems to be empty. With the following patch I get at least > consistently: > [...] > Not sure however whether the stack is supposed to be empty here... Eyeballing the code it *seems* to me that: * The constructor will create a DeepIterator with an empty stack if there is no non-widget content in the page. In that case it sets hasContents = false . * In all other cases the stack will always remain non-empty, and the top iterator will always point to the same widget it was created with. * Your fix for the DeepIterator search methods is the correct fix for the original problem. * The correct fix for *this* problem is for SelectionState objects to check any DeepIterator objects they create and reset the link/selection if the new DeepIterator object is empty. This is because the methods that adjust selections assume that the from and to members are either NULL or non-empty. Let me work on a patch for this. Comments? Jeremy Henty

Dec. 7, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

On Fri, Dec 07, 2007 at 08:45:00AM +0100, Johannes Hofmann wrote: > The stack seems to be empty. With the following patch I get at least > consistently: > > (gdb) p top > $1 = (class dw::core::Iterator *) 0x0 > [snip] OK, thanks for that. I'll try getting my head round iterators today. > PS: Compiling the whole thing with -fno-inline makes debugging much > easier. I'm already using -g -O0 as the valgrind docs recommend. Doesn't -O0 imply -fno-inline? The gcc info describes -O0 as "Do not optimize." and says under -fno-inline "Note that if you are not optimizing, no functions can be expanded inline." which suggests that -O0 makes -fno-inline redundant. I certainly noticed functions disappearing from the stack trace before I selected -O0. Regards, Jeremy Henty

Dec. 7, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

On Thu, Dec 06, 2007 at 06:12:47PM +0000, Jeremy Henty wrote: > On Thu, Dec 06, 2007 at 05:19:34PM +0100, Johannes Hofmann wrote: > > > [snip] > > fixes the crashes for me. Please test. > > Possibly only a partial fix, I'm afraid. The crash on the page I sent > you has gone, but I have a test file containing just an image, and > when I click to the right of that image dillo segfaults. gdb says: > > Program terminated with signal 11, Segmentation fault. > #0 0x080989c3 in dw::core::SelectionState::correctCharPos (it=0x810dfe8, > charPos=1073741824) at selection.cc:362 > 362 if (top->getContent()->type == Content::TEXT) > (gdb) print top > $1 = (class dw::core::Iterator *) 0x19 > (gdb) print top->getContent() > Cannot access memory at address 0x19 > (gdb) print it > $2 = (class dw::core::DeepIterator *) 0x810dfe8 > > That 0x19 pointer looks like memory corruption, and valgrind confirms > that dillo is accessing freed memory. I'll dig into its output and > see what sense I can make. The stack seems to be empty. With the following patch I get at least consistently: (gdb) p top $1 = (class dw::core::Iterator *) 0x0 diff -r 7817ef0b4b13 lout/container.hh --- a/lout/container.hh Fri Dec 07 08:33:56 2007 +0100 +++ b/lout/container.hh Fri Dec 07 08:38:50 2007 +0100 @@ -116,7 +116,7 @@ public: void insert(object::Object *newElement, int pos); void remove(int pos); inline object::Object *get(int pos) - { return pos < numElements ? array[pos] : NULL; } + { return (pos >= 0 && pos < numElements) ? array[pos] : NULL; } inline int size() { return numElements; } void clear(); void sort(); Not sure however whether the stack is supposed to be empty here... Regards, Johannes PS: Compiling the whole thing with -fno-inline makes debugging much easier. > > Of course this might be a completely different bug from the one your > patch fixes. Perhaps you could assume that for the moment and commit > anyway? > > Regards, > > Jeremy Henty > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 7, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

On Thu, Dec 06, 2007 at 06:12:47PM +0000, Jeremy Henty wrote: > On Thu, Dec 06, 2007 at 05:19:34PM +0100, Johannes Hofmann wrote: > > > [snip] > > fixes the crashes for me. Please test. > > Possibly only a partial fix, I'm afraid. The crash on the page I sent > you has gone, but I have a test file containing just an image, and > when I click to the right of that image dillo segfaults. gdb says: > > Program terminated with signal 11, Segmentation fault. > #0 0x080989c3 in dw::core::SelectionState::correctCharPos (it=0x810dfe8, > charPos=1073741824) at selection.cc:362 > 362 if (top->getContent()->type == Content::TEXT) > (gdb) print top > $1 = (class dw::core::Iterator *) 0x19 > (gdb) print top->getContent() > Cannot access memory at address 0x19 > (gdb) print it > $2 = (class dw::core::DeepIterator *) 0x810dfe8 > > That 0x19 pointer looks like memory corruption, and valgrind confirms > that dillo is accessing freed memory. I'll dig into its output and > see what sense I can make. I can reproduce this here. But I won't have much time the next days. Regards, Johannes > > Of course this might be a completely different bug from the one your > patch fixes. Perhaps you could assume that for the moment and commit > anyway? > > Regards, > > Jeremy Henty > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 6, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

On Thu, Dec 06, 2007 at 05:19:34PM +0100, Johannes Hofmann wrote: > [snip] > fixes the crashes for me. Please test. Possibly only a partial fix, I'm afraid. The crash on the page I sent you has gone, but I have a test file containing just an image, and when I click to the right of that image dillo segfaults. gdb says: Program terminated with signal 11, Segmentation fault. #0 0x080989c3 in dw::core::SelectionState::correctCharPos (it=0x810dfe8, charPos=1073741824) at selection.cc:362 362 if (top->getContent()->type == Content::TEXT) (gdb) print top $1 = (class dw::core::Iterator *) 0x19 (gdb) print top->getContent() Cannot access memory at address 0x19 (gdb) print it $2 = (class dw::core::DeepIterator *) 0x810dfe8 That 0x19 pointer looks like memory corruption, and valgrind confirms that dillo is accessing freed memory. I'll dig into its output and see what sense I can make. Of course this might be a completely different bug from the one your patch fixes. Perhaps you could assume that for the moment and commit anyway? Regards, Jeremy Henty

Dec. 6, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

Ok, I think I found it. At least diff -r 8edae7f02bd7 dw/iterator.cc --- a/dw/iterator.cc Thu Dec 06 13:30:01 2007 +0100 +++ b/dw/iterator.cc Thu Dec 06 17:16:42 2007 +0100 @@ -343,7 +343,7 @@ Iterator *DeepIterator::searchDownward ( return NULL; } - while (fromEnd ? it->prev () : it->next ()) { + while (fromEnd ? it2->prev () : it2->next ()) { //DEBUG_MSG (1, "%*sexamining %s\n", // indent, "", a_Dw_iterator_text (it2)); @@ -351,7 +351,7 @@ Iterator *DeepIterator::searchDownward ( // Another widget. Search in it downwards. it3 = searchDownward (it2, mask, fromEnd); if (it3 != NULL) { - it->unref (); + it2->unref (); return it3; } // Else continue in this widget. @@ -384,7 +384,7 @@ Iterator *DeepIterator::searchSideward ( misc::assert (it->getContent()->type == Content::WIDGET, "no widget"); it2 = it->cloneIterator (); - while (fromEnd ? it->prev () : it->next ()) { + while (fromEnd ? it2->prev () : it2->next ()) { if (it2->getContent()->type == Content::WIDGET) { // Search downwards in this widget. it3 = searchDownward (it2, mask, fromEnd); fixes the crashes for me. Please test. Cheers, Johannes On Thu, Dec 06, 2007 at 03:26:40PM +0100, Johannes Hofmann wrote: > Hi Jeremy, > > On Wed, Dec 05, 2007 at 11:45:53PM +0000, Jeremy Henty wrote: > > > > Dillo aborts with an assertion failure if you click to the right of an > > image that has a line all to itself. For example, go to > > http://www.thenation.com/doc/20071203/pollitt and click to the right > > of the "Continued" logo. > > Great you are looking into that. It was next on my list... > > > > > What happens is that the DeepIterator constructor wants an iterator > > with non-widget content. If necessary it calls searchDownward and > > searchSideward until it finds one. Both these methods raise an > > assertion failure if the iterator already has non-widget content, so > > the content must be checked before calling *either* of them. However, > > DeepIterator checks the content before calling searchDownward but > > *not* before calling searchSideward . Effectively, the code assumes > > that if searchDownward fails then the content type won't have changed. > > > > Unfortunately searchDownward can fail *and* change the content of the > > iterator to something non-widget. If this happens DeepIterator then > > calls searchSideward, which triggers the assertion. > > > > (NB: DeepIterator is in the dw::core namespace.) > > I think both DeepIterator::searchDownward() and > DeepIterator::searchSideward() are not supposed to modify their > argument it. > > So without really understanding what's going on, I would say it's a > typo and should read: > > diff -r 8edae7f02bd7 dw/iterator.cc > --- a/dw/iterator.cc Thu Dec 06 13:30:01 2007 +0100 > +++ b/dw/iterator.cc Thu Dec 06 15:23:20 2007 +0100 > @@ -343,7 +343,7 @@ Iterator *DeepIterator::searchDownward ( > return NULL; > } > > - while (fromEnd ? it->prev () : it->next ()) { > + while (fromEnd ? it2->prev () : it2->next ()) { > //DEBUG_MSG (1, "%*sexamining %s\n", > // indent, "", a_Dw_iterator_text (it2)); > > @@ -384,7 +384,7 @@ Iterator *DeepIterator::searchSideward ( > misc::assert (it->getContent()->type == Content::WIDGET, "no > widget"); > it2 = it->cloneIterator (); > > - while (fromEnd ? it->prev () : it->next ()) { > + while (fromEnd ? it2->prev () : it2->next ()) { > if (it2->getContent()->type == Content::WIDGET) { > // Search downwards in this widget. > it3 = searchDownward (it2, mask, fromEnd); > > > > Unfortunately it now crashes somewhere else related to text selection... > > Regards, > Johannes > > > > > I'm trying to fix this but so far I'm just generating more crashes. > > What exactly should DeepIterator::DeepIterator do if searchDownward > > fails and changes the content of the iterator? If I know that I'll > > have another go. > > > > Regards, > > > > Jeremy Henty > > > > _______________________________________________ > > Dillo-dev mailing list > > Dillo-dev(a)dillo.org > > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 6, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by Johannes.Hofmann＠gmx.de

Hi Jeremy, On Wed, Dec 05, 2007 at 11:45:53PM +0000, Jeremy Henty wrote: > > Dillo aborts with an assertion failure if you click to the right of an > image that has a line all to itself. For example, go to > http://www.thenation.com/doc/20071203/pollitt and click to the right > of the "Continued" logo. Great you are looking into that. It was next on my list... > > What happens is that the DeepIterator constructor wants an iterator > with non-widget content. If necessary it calls searchDownward and > searchSideward until it finds one. Both these methods raise an > assertion failure if the iterator already has non-widget content, so > the content must be checked before calling *either* of them. However, > DeepIterator checks the content before calling searchDownward but > *not* before calling searchSideward . Effectively, the code assumes > that if searchDownward fails then the content type won't have changed. > > Unfortunately searchDownward can fail *and* change the content of the > iterator to something non-widget. If this happens DeepIterator then > calls searchSideward, which triggers the assertion. > > (NB: DeepIterator is in the dw::core namespace.) I think both DeepIterator::searchDownward() and DeepIterator::searchSideward() are not supposed to modify their argument it. So without really understanding what's going on, I would say it's a typo and should read: diff -r 8edae7f02bd7 dw/iterator.cc --- a/dw/iterator.cc Thu Dec 06 13:30:01 2007 +0100 +++ b/dw/iterator.cc Thu Dec 06 15:23:20 2007 +0100 @@ -343,7 +343,7 @@ Iterator *DeepIterator::searchDownward ( return NULL; } - while (fromEnd ? it->prev () : it->next ()) { + while (fromEnd ? it2->prev () : it2->next ()) { //DEBUG_MSG (1, "%*sexamining %s\n", // indent, "", a_Dw_iterator_text (it2)); @@ -384,7 +384,7 @@ Iterator *DeepIterator::searchSideward ( misc::assert (it->getContent()->type == Content::WIDGET, "no widget"); it2 = it->cloneIterator (); - while (fromEnd ? it->prev () : it->next ()) { + while (fromEnd ? it2->prev () : it2->next ()) { if (it2->getContent()->type == Content::WIDGET) { // Search downwards in this widget. it3 = searchDownward (it2, mask, fromEnd); Unfortunately it now crashes somewhere else related to text selection... Regards, Johannes > > I'm trying to fix this but so far I'm just generating more crashes. > What exactly should DeepIterator::DeepIterator do if searchDownward > fails and changes the content of the iterator? If I know that I'll > have another go. > > Regards, > > Jeremy Henty > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 6, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by jcid＠dillo.org

Hi, On Wed, Dec 05, 2007 at 11:45:53PM +0000, Jeremy Henty wrote: > > Dillo aborts with an assertion failure if you click to the right of an > image that has a line all to itself. For example, go to > http://www.thenation.com/doc/20071203/pollitt and click to the right > of the "Continued" logo. > > What happens is that the DeepIterator constructor wants an iterator > with non-widget content. If necessary it calls searchDownward and > searchSideward until it finds one. Both these methods raise an > assertion failure if the iterator already has non-widget content, so > the content must be checked before calling *either* of them. However, > DeepIterator checks the content before calling searchDownward but > *not* before calling searchSideward . Effectively, the code assumes > that if searchDownward fails then the content type won't have changed. > > Unfortunately searchDownward can fail *and* change the content of the > iterator to something non-widget. If this happens DeepIterator then > calls searchSideward, which triggers the assertion. > > (NB: DeepIterator is in the dw::core namespace.) > > I'm trying to fix this but so far I'm just generating more crashes. > What exactly should DeepIterator::DeepIterator do if searchDownward > fails and changes the content of the iterator? If I know that I'll > have another go. Sorry, I can't help you on this one. Sebastian may answer later, or you can continue your research on the problem... -- Cheers Jorge.-

Dec. 6, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by jcid＠dillo.org

On Wed, Dec 05, 2007 at 06:18:22PM +0000, Jeremy Henty wrote: > On Wed, Dec 05, 2007 at 12:34:18PM -0300, Jorge Arellano Cid wrote: > > > The problem is reentrancy. > > Aha! I did some testing and noticed that calls to a_IO_ccc were > sometimes nested. > > > This bug was produced because a CCC operation was reentering the > > same chain after it was aborted. I added an extra check as a > > workaround to avoid this situation and a comment that a more generic > > solution should be designed. > > Hmm, perhaps the CCC Chainlink structures could have an aborted flag > and the chaining functions could call the callbacks only if the link > was not aborted? Yes. I gave that idea a quick try and realized I had to decide better whether to allow some reentrancy, why/when and how, and also to check if that was already happening... > Also, I noticed that a_Chain_unlink is never used. Is that > intentional? It's just an API idea. -- Cheers Jorge.-

Dec. 6, 2007

[patch] fix crashes after panel size changes

by jcid＠dillo.org

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.-

Dec. 6, 2007

[patch] fix crashes after panel size changes

by Johannes.Hofmann＠gmx.de

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? 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. 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(). Does anyone remember why FltkWidgetView::prepareCreateFltkWidget() was added in the first place? Cheers, Johannes diff --git a/dw/fltkplatform.cc b/dw/fltkplatform.cc --- a/dw/fltkplatform.cc +++ b/dw/fltkplatform.cc @@ -83,10 +83,6 @@ FltkColor::FltkColor (int color, core::s colors[SHADING_DARK] = shadeColor (color, SHADING_DARK) << 8; colors[SHADING_LIGHT] = shadeColor (color, SHADING_LIGHT) << 8; } -} - -void FltkView::prepareCreateFltkWidget () -{ } void FltkView::addFltkWidget (::fltk::Widget *widget, diff --git a/dw/fltkplatform.hh b/dw/fltkplatform.hh --- a/dw/fltkplatform.hh +++ b/dw/fltkplatform.hh @@ -40,7 +40,6 @@ public: public: virtual bool usesFltkWidgets () = 0; - virtual void prepareCreateFltkWidget (); virtual void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); virtual void removeFltkWidget (::fltk::Widget *widget); diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -103,7 +103,6 @@ void FltkResource::attachView (FltkView ViewAndWidget *viewAndWidget = new ViewAndWidget(); viewAndWidget->view = view; - view->prepareCreateFltkWidget (); viewAndWidget->widget = createNewWidget (&allocation); viewAndWidget->view->addFltkWidget (viewAndWidget->widget, &allocation); viewsAndWidgets->append (viewAndWidget); diff --git a/dw/fltkviewbase.cc b/dw/fltkviewbase.cc --- a/dw/fltkviewbase.cc +++ b/dw/fltkviewbase.cc @@ -375,17 +375,12 @@ bool FltkWidgetView::usesFltkWidgets () return true; } - -void FltkWidgetView::prepareCreateFltkWidget () -{ - begin (); -} - void FltkWidgetView::addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation) { canvasWidgets->append (new TypedPointer < ::fltk::Widget> (widget)); allocateFltkWidget (widget, allocation); + add (widget); } void FltkWidgetView::removeFltkWidget (::fltk::Widget *widget) diff --git a/dw/fltkviewbase.hh b/dw/fltkviewbase.hh --- a/dw/fltkviewbase.hh +++ b/dw/fltkviewbase.hh @@ -82,7 +82,6 @@ public: int x, int y, int width, int height); bool usesFltkWidgets (); - void prepareCreateFltkWidget (); void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); void removeFltkWidget (::fltk::Widget *widget); void allocateFltkWidget (::fltk::Widget *widget,

Dec. 6, 2007

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward

by onepoint＠starurchin.org

Dillo aborts with an assertion failure if you click to the right of an image that has a line all to itself. For example, go to http://www.thenation.com/doc/20071203/pollitt and click to the right of the "Continued" logo. What happens is that the DeepIterator constructor wants an iterator with non-widget content. If necessary it calls searchDownward and searchSideward until it finds one. Both these methods raise an assertion failure if the iterator already has non-widget content, so the content must be checked before calling *either* of them. However, DeepIterator checks the content before calling searchDownward but *not* before calling searchSideward . Effectively, the code assumes that if searchDownward fails then the content type won't have changed. Unfortunately searchDownward can fail *and* change the content of the iterator to something non-widget. If this happens DeepIterator then calls searchSideward, which triggers the assertion. (NB: DeepIterator is in the dw::core namespace.) I'm trying to fix this but so far I'm just generating more crashes. What exactly should DeepIterator::DeepIterator do if searchDownward fails and changes the content of the iterator? If I know that I'll have another go. Regards, Jeremy Henty

Dec. 6, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

On Wed, Dec 05, 2007 at 12:34:18PM -0300, Jorge Arellano Cid wrote: > The problem is reentrancy. Aha! I did some testing and noticed that calls to a_IO_ccc were sometimes nested. > This bug was produced because a CCC operation was reentering the > same chain after it was aborted. I added an extra check as a > workaround to avoid this situation and a comment that a more generic > solution should be designed. Hmm, perhaps the CCC Chainlink structures could have an aborted flag and the chaining functions could call the callbacks only if the link was not aborted? Also, I noticed that a_Chain_unlink is never used. Is that intentional? > Patch is in CVS. Problem solved. Yay! Regards, Jeremy Henty

Dec. 5, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by jcid＠dillo.org

Hi Jeremy, On Tue, Dec 04, 2007 at 02:03:07PM +0000, Jeremy Henty wrote: > > I am hoping that someone who understands Dillo's CCC better than I can > point me in the right direction and save me some time debugging this. Well, ehmmmm, I know CCC, but this was a hard-to-find bug! > Dillo almost always crashes if the HTML contains the stumbleupon.com > favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. > (This is very annoying as it makes Dillo unusable for one of my > favourite blogs.) If it doesn't crash right away then clicking Reload > crashes it. > > Examining core dumps with gdb suggests that memory is being corrupted, > and running dillo under valgrind shows that the CCC functions are > reading, writing and freeing already freed memory. (Details > attached.) > > A local HTTP server triggers the bug when it server returns a > Content-Type of "text/plain; charset=iso-8859-1". If I change it to > "image/x-icon" there is no corruption. Accessing the image directly > via a "file:" URL does not trigger the bug. (Strangely, using a local > HTTP server does not hit the bug until I open a second window on the > page, although sometimes it crashes first time.) > > I'm attaching a summary of the invalid memory accesses that valgrind > reported, and the entire valgrind log (which includes stack traces). > > Any suggestions for where I should start instrumenting the CCC to find > out why it is prematurely freeing pointers? The problem is reentrancy. The CCC handles parallelism and serves to propagate operations in a managed environment. This bug was produced because a CCC operation was reentering the same chain after it was aborted. I added an extra check as a workaround to avoid this situation and a comment that a more generic solution should be designed. Patch is in CVS. Please test and comment. Hopefully it will work. ;-) -- Cheers Jorge.-

Dec. 5, 2007

[patch] initialize mustQueueResize in Textblock constructor

by jcid＠dillo.org

On Wed, Dec 05, 2007 at 10:59:24AM +0100, Johannes Hofmann wrote: > Hello, > > this patch initializes mustQueueResize in the Textblock constructor. > Valgrind was complaining about a jump depending on uninitialized > memory. Committed. -- Cheers Jorge.-

Dec. 5, 2007

[patch] add missing style->ref ()

by jcid＠dillo.org

On Wed, Dec 05, 2007 at 10:57:04AM +0100, Johannes Hofmann wrote: > Hello, > > the tiny patch below adds a missing style->ref() which was causing > a lot of crashes on my system. Committed. -- Cheers Jorge.-

Dec. 5, 2007

[patch] initialize mustQueueResize in Textblock constructor

by Johannes.Hofmann＠gmx.de

Hello, this patch initializes mustQueueResize in the Textblock constructor. Valgrind was complaining about a jump depending on uninitialized memory. Cheers, Johannes diff --git a/dw/textblock.cc b/dw/textblock.cc --- a/dw/textblock.cc +++ b/dw/textblock.cc @@ -39,6 +39,7 @@ Textblock::Textblock (bool limitTextWidt line1Offset = 0; line1OffsetEff = 0; ignoreLine1OffsetSometimes = false; + mustQueueResize = false; /* * The initial sizes of lines and words should not be

Dec. 5, 2007

[patch] add missing style->ref ()

by Johannes.Hofmann＠gmx.de

Hello, the tiny patch below adds a missing style->ref() which was causing a lot of crashes on my system. cheers, Johannes diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -159,6 +159,7 @@ void FltkResource::setStyle (core::style this->style->unref (); this->style = style; + style->ref (); for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); it.hasNext(); ) {

Dec. 5, 2007

[patch] partial redraws - updated

by jcid＠dillo.org

Hi, On Tue, Dec 04, 2007 at 02:42:32PM +0100, Johannes Hofmann wrote: > Hi Jorge, > > > > > Committed! > > > > > > Thanks. Attached is another patch that has some improvements to that > > > code: > > > > Excellent. > > > > Committed. > > until now, just the Changlog has been commited, right? Right, my mistake. :-P Committed again. > > If not, even when I'm not an expert in dw2, I'd prefer to > > push forward development in that area and to commit what looks > > OK. Sebastian will always have the opportunity to comment/fix > > later. > > Regarding the scrollbars there is something fishy in the current > code that needs fixing: The scrollbars are created in the > FltkViewport constructor and they are drawn on the screen area > belonging to FltkViewport. However from fltk point of view they are > children of TopGroup. Yes, we know there's something fishy there because while in Germany we discovered scrollbars worked without the line that was supposedly drawing them! :-) > I think this is the reason why fltk replace() does not work > properly. > > However, my proposed fix is really quite radical and I would want to > wait for a comment from Sebastian before we head into that > direction. OK. > > As a matter of fact I'm working of improving table rendering > > now, and I already have some code that works. It's a complex area > > that needs some work but I hope to commit a patch this week. > > > > BTW, did you receive comments on the patch that lowered CPU > > consumption when gliding the mouse over pages full of links? > > No, but the change is not that dramatic, so I think it can be > applied. Do you need a patch against current cvs? Committed. -- Cheers Jorge.-

Dec. 4, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

Heh, I sent the valgrind logs separately but they were so large they were held for moderation. This is just the summarised version. It lists the freed blocks that were accessed and the sequence of invalid accesses. It's clear that the CCC is accessing pointers after freeing them. Jeremy Henty Freed blocks: [1]: 0x4AB7910:(IOData_t *), freed by a_IO_ccc (IO.c:385) [2]: 0x4AB78C0:(ChainLink *Info), freed by a_IO_ccc (IO.c:386) [3]: 0x4AB5768:(ChainLink *Info), freed by a_Dpi_ccc (dpi.c:696) [4]: 0x4AB5718:(ChainLink *Info), freed by a_Capi_ccc (capi.c:536) [5]: 0x4AACE10:(capi_conn_t *), freed by a_Capi_ccc (capi.c:535) [6]: 0x4AACE68:(DilloUrl *), freed by a_Capi_ccc (capi.c:535) [7]: 0x4AB57B8:(dpi_conn_t *), freed by a_Dpi_ccc (dpi.c:695) [8]: 0x4AB5818:(Dstr *), freed by a_Dpi_ccc (dpi.c:695) [9]: 0x4AB5858:(Dstr_char_t *), freed by a_Dpi_ccc (dpi.c:672) [10]: 0x4AB7960:(Dstr *ds), freed by a_IO_ccc (IO.c:385) [11]: 0x4AB79A0:(Dstr_char_t *), freed by a_IO_ccc (IO.c:385) [1] read at IO_read (IO.c:194) [2] read at a_IO_ccc (IO.c:402) (many times) [3] read at a_IO_ccc (IO.c:402) (many times) [4] read at a_Capi_ccc (capi.c:569) [5] write at a_Capi_ccc (capi.c:570) [5] read at a_Capi_ccc (capi.c:572) [6] read at a_Url_cmp (url.c:442) [5] read at a_Capi_ccc (capi.c:574) [5] read at Capi_conn_unref (capi.c:132) (twice) [4] freed by a_Capi_ccc (capi.c:579) [3] read at a_Dpi_ccc (dpi.c:672) [7] read at Dpi_conn_free (dpi.c:124) [7] read at Dpi_conn_free (dpi.c:125) [8] read at dStr_free (dlib.c:291) [8] read at dStr_free (dlib.c:292) [9] freed by a_Dpi_ccc (dpi.c:672) [8] freed by a_Dpi_ccc (dpi.c:672) [7] freed by a_Dpi_ccc (dpi.c:672) [3] freed by a_Dpi_ccc (dpi.c:673) [1] read at IO_read (IO.c:194) (many times) [10] read at dStr_free (dlib.c:291) [1] read at dStr_free (dlib.c:292) [11] freed by a_IO_ccc (IO.c:404) [10] freed by a_IO_ccc (IO.c:404) [1] freed by a_IO_ccc (IO.c:404) [2] freed by a_IO_ccc (IO.c:405)

Dec. 4, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

I am hoping that someone who understands Dillo's CCC better than I can point me in the right direction and save me some time debugging this. Dillo almost always crashes if the HTML contains the stumbleupon.com favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. (This is very annoying as it makes Dillo unusable for one of my favourite blogs.) If it doesn't crash right away then clicking Reload crashes it. Examining core dumps with gdb suggests that memory is being corrupted, and running dillo under valgrind shows that the CCC functions are reading, writing and freeing already freed memory. (Details attached.) A local HTTP server triggers the bug when it server returns a Content-Type of "text/plain; charset=iso-8859-1". If I change it to "image/x-icon" there is no corruption. Accessing the image directly via a "file:" URL does not trigger the bug. (Strangely, using a local HTTP server does not hit the bug until I open a second window on the page, although sometimes it crashes first time.) I'm attaching a summary of the invalid memory accesses that valgrind reported, and the entire valgrind log (which includes stack traces). Any suggestions for where I should start instrumenting the CCC to find out why it is prematurely freeing pointers? Jeremy Henty

Dec. 4, 2007

[patch] partial redraws - updated

by Johannes.Hofmann＠gmx.de

Hi Jorge, On Tue, Dec 04, 2007 at 10:05:25AM -0300, Jorge Arellano Cid wrote: > Hi Johannes, > > On Tue, Dec 04, 2007 at 11:03:01AM +0100, Johannes Hofmann wrote: > > On Mon, Dec 03, 2007 at 02:00:07PM -0300, Jorge Arellano Cid wrote: > > > On Sun, Dec 02, 2007 at 01:53:14PM +0100, Johannes Hofmann wrote: > > > > Hello, > > > > > > > > this patch fixes the problem pointed out by Sebastian regarding > > > > partial redraws and scrolling interference. > > > > It adds a new Region class that is implemented similarily > > > > as in dillo1 as a set of rectangles not containing each other. > > > > The conversion from canvas to view coordinates is now done at > > > > draw-time. > > > > > > Committed! > > > > Thanks. Attached is another patch that has some improvements to that > > code: > > Excellent. > > Committed. until now, just the Changlog has been commited, right? > > > * The case that multiple rectangles can be exactly combined to a larger > > one is quite likely (other than the comment in dillo1 suggests). > > It happens all the time, e.g. when an image is loaded piece by piece. > > The region code is improved to take advantage of this. > > ok. > > > * A minor bug also present in the dillo1 code is fixed: If a > > rectangle existing already in the region is contained in a newly > > added rectangle, we must not stop the loop. The new rectangle may > > contain other rectangles as well! > > ok. > > > * The actual drawing of a region now checks, whether a rectangle is > > actually visible at the moment. > > Does this help to reduce flickering? I don't think so, it just shortcuts drawing operations that are not visible anyway. > > > * The background is now cleared properly when doing a partial > > redraw. This fixes some screen corruption occuring when selecting / > > deselecting text. > > ok. > > Please excuse me for not having yet the time to review the > patch for the scrolling bars. Has Sebastian commented it to you? No, not yet. > > If not, even when I'm not an expert in dw2, I'd prefer to > push forward development in that area and to commit what looks > OK. Sebastian will always have the opportunity to comment/fix > later. Regarding the scrollbars there is something fishy in the current code that needs fixing: The scrollbars are created in the FltkViewport constructor and they are drawn on the screen area belonging to FltkViewport. However from fltk point of view they are children of TopGroup. I think this is the reason why fltk replace() does not work properly. However, my proposed fix is really quite radical and I would want to wait for a comment from Sebastian before we head into that direction. > > As a matter of fact I'm working of improving table rendering > now, and I already have some code that works. It's a complex area > that needs some work but I hope to commit a patch this week. > > BTW, did you receive comments on the patch that lowered CPU > consumption when gliding the mouse over pages full of links? No, but the change is not that dramatic, so I think it can be applied. Do you need a patch against current cvs? Cheers, Johannes > > -- > Cheers > Jorge.- > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 4, 2007

[patch] partial redraws - updated

by jcid＠dillo.org

Hi Johannes, On Tue, Dec 04, 2007 at 11:03:01AM +0100, Johannes Hofmann wrote: > On Mon, Dec 03, 2007 at 02:00:07PM -0300, Jorge Arellano Cid wrote: > > On Sun, Dec 02, 2007 at 01:53:14PM +0100, Johannes Hofmann wrote: > > > Hello, > > > > > > this patch fixes the problem pointed out by Sebastian regarding > > > partial redraws and scrolling interference. > > > It adds a new Region class that is implemented similarily > > > as in dillo1 as a set of rectangles not containing each other. > > > The conversion from canvas to view coordinates is now done at > > > draw-time. > > > > Committed! > > Thanks. Attached is another patch that has some improvements to that > code: Excellent. Committed. > * The case that multiple rectangles can be exactly combined to a larger > one is quite likely (other than the comment in dillo1 suggests). > It happens all the time, e.g. when an image is loaded piece by piece. > The region code is improved to take advantage of this. ok. > * A minor bug also present in the dillo1 code is fixed: If a > rectangle existing already in the region is contained in a newly > added rectangle, we must not stop the loop. The new rectangle may > contain other rectangles as well! ok. > * The actual drawing of a region now checks, whether a rectangle is > actually visible at the moment. Does this help to reduce flickering? > * The background is now cleared properly when doing a partial > redraw. This fixes some screen corruption occuring when selecting / > deselecting text. ok. Please excuse me for not having yet the time to review the patch for the scrolling bars. Has Sebastian commented it to you? If not, even when I'm not an expert in dw2, I'd prefer to push forward development in that area and to commit what looks OK. Sebastian will always have the opportunity to comment/fix later. As a matter of fact I'm working of improving table rendering now, and I already have some code that works. It's a complex area that needs some work but I hope to commit a patch this week. BTW, did you receive comments on the patch that lowered CPU consumption when gliding the mouse over pages full of links? -- Cheers Jorge.-

Dec. 4, 2007