- Dillo-dev - lists.mailman3.com

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