- Dillo-dev - lists.mailman3.com

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