- Dillo-dev - lists.mailman3.com

Re: patch: escape for display

by jcid＠dillo.org

On Sun, Apr 06, 2008 at 11:08:13PM +0000, corvid wrote: > > I ponder having a: > > > > const char *dStr_printable(dStr *in, int maxlen) > > > > that returns a pointer to an internal static dStr. It has > > the advantage of minimal interface overhead. e.g. > > > > - MSG_WARN(" \"%s\"\n", io->Buf->str); > > + MSG_WARN(" \"%s\"\n", dStr_printable(io->Buf, 2048)); > > > > of course it's not MT-safe, but we don't need it now. > > > > Implementing a MT-safe version is easy, but involves more > > calling overhead: > > > > dStr *dStr_printable(dStr *in, int maxlen) > > > > - MSG_WARN(" \"%s\"\n", io->Buf->str); > > + Dstr *printable = dStr_printable(io->Buf, 2048)); > > + MSG_WARN(" \"%s\"\n", printable->str)); > > + dStr_free(printable, 1); > > > > and disabling the message is not as clean as prepending an "_" > > to the macro. > > > > I'd go with the first alternative. > > Attached. > > I wasn't too sure about representation. Text has \x0D where ^M would > be much less surprising, but \x is easier to read for arbitrary bytes. > > It's funny that I had been calling truncation a bug and now I'm > truncating intentionally, but there is only so much a person > wants to see... Committed. -- Cheers Jorge.-

April 7, 2008

[BUG]: anchors break when a page includes '<meta http-equiv="Content-Type" ...'

by jcid＠dillo.org

Hi, On Sat, Apr 05, 2008 at 11:58:45AM +0200, Johannes Hofmann wrote: > [...] > I noticed one regression with the new anchor code: > The "Continue Reading Older Posts" link on www.boingboing.net > (e.g. http://www.boingboing.net/2008/04/03/#entry-44240) no longer > scrolls to the right position. I checked with a dillo version even as old as 2007-11-23 and the bug is present. So it's not related to the newest anchor patch. It seems somewhere the offset is not being updated. If I load the page from cache, it works! i.e. file:/tmp/44240.html file:/tmp/44240.html#entry-44240 works OK. Using a cgi to simulate network transfer rate (10KB/sec), I can reproduce the bug: http://localhost/cgi-bin/slow.cgi?10000:/tmp/44240.html#entry-44240 That's what I've found so far. -- Cheers Jorge.-

April 7, 2008

Re: patch: dStr_memmem

by jcid＠dillo.org

Hi, Committed with a minor modification: /* * Return a pointer to the first occurrence of needle in haystack. */ char *dStr_memmem(Dstr *haystack, Dstr *needle) { int i; if (needle && haystack) { if (needle->len == 0) return haystack->str; for (i = 0; i <= (haystack->len - needle->len); i++) { - if (!memcmp(haystack->str + i, needle->str, needle->len)) + if (haystack->str[i] == needle->str[0] && + !memcmp(haystack->str + i, needle->str, needle->len)) return haystack->str + i; } } return NULL; } The idea is to avoid a 3 parameters function call per each checked character. Maybe it's even better to implement it without a function call (like dStristr()), but I haven't done any profiling so I'm not sure. It looks like an improvement though. ;-) -- Cheers Jorge.-

April 6, 2008

Re: Good news from fltk!

by Johannes.Hofmann＠gmx.de

Hi, my first patch broke test programs in dw2/test/ (as they don't use clear_double_buffer() as dillo-fltk does). Corrected patch attached. Sorry for that, Johannes On Sat, Apr 05, 2008 at 05:01:52PM +0200, Johannes Hofmann wrote: > Hi, > > I stumbled across this post on fltk.bugs: > http://fltk.org/newsgroups.php?s5281+gfltk.bugs+v5290+T0 > > This is great for us, as it allows us to implement efficient double > buffering without X11 specific hacks. > Attached is a patch that enables double buffering for partial screen > redraws (this is where the flickering is most annoying). > You will need at least version r6079 of fltk2 to test. > > Cheers, > Johannes > diff -r 0949076de1d4 configure.in > --- a/configure.in Sat Apr 05 13:33:25 2008 +0200 > +++ b/configure.in Sat Apr 05 16:48:27 2008 +0200 > @@ -21,6 +21,7 @@ AC_ARG_ENABLE(ansi, [ --enable-ansi > AC_ARG_ENABLE(ansi, [ --enable-ansi Try to compile and run with ANSI flags], > , enable_ansi=no) > AC_ARG_ENABLE(rtfl, [ --enable-rtfl Build with rtfl messages], enable_rtfl=yes) > +AC_ARG_ENABLE(doublebuffer, [ --disable-doublebuffer Disable double buffering for drawing], disable_doublebuffer=yes) > > AC_PROG_CXX > AM_PROG_CC_STDC > @@ -61,6 +62,9 @@ if test "x$enable_rtfl" = "xyes" ; then > if test "x$enable_rtfl" = "xyes" ; then > CXXFLAGS="$CXXFLAGS -DDBG_RTFL" > fi > +if test "x$disable_doublebuffer" = "xyes" ; then > + CXXFLAGS="$CXXFLAGS -DNO_DOUBLEBUFFER" > +fi > > dnl ----------------------- > dnl Checks for header files > diff -r 0949076de1d4 dw/fltkviewbase.cc > --- a/dw/fltkviewbase.cc Sat Apr 05 13:33:25 2008 +0200 > +++ b/dw/fltkviewbase.cc Sat Apr 05 16:48:27 2008 +0200 > @@ -38,6 +38,8 @@ namespace dw { > namespace dw { > namespace fltk { > > +::fltk::Image *FltkViewBase::backBuffer; > + > FltkViewBase::FltkViewBase (int x, int y, int w, int h, const char *label): > Group (x, y, w, h, label) > { > @@ -46,13 +48,17 @@ FltkViewBase::FltkViewBase (int x, int y > bgColor = WHITE; > lastDraw = time(0); > drawDelay = 2; /* in seconds */ > +#ifndef NO_DOUBLEBUFFER > + if (!backBuffer) { > + backBuffer = new Image (); > + } > +#endif > } > > FltkViewBase::~FltkViewBase () > { > cancelQueueDraw (); > } > - > > void FltkViewBase::draw () > { > @@ -78,9 +84,27 @@ void FltkViewBase::draw () > > if (!viewRect.empty ()) { > push_clip (viewRect); > +#ifndef NO_DOUBLEBUFFER > + { > + GSave gsave; > + > + backBuffer->setsize (viewRect.w (), viewRect.h ()); > + backBuffer->make_current (); > + translate (-viewRect.x () , -viewRect.y ()); > + > + setcolor (bgColor); > + fillrect (viewRect); > + theLayout->expose (this, rect); > + } > + > + make_current (); > + backBuffer->draw (Rectangle (0, 0, viewRect.w (), viewRect.h ()), > + viewRect); > +#else > setcolor (bgColor); > fillrect (viewRect); > theLayout->expose (this, rect); > +#endif > pop_clip (); > } > } > diff -r 0949076de1d4 dw/fltkviewbase.hh > --- a/dw/fltkviewbase.hh Sat Apr 05 13:33:25 2008 +0200 > +++ b/dw/fltkviewbase.hh Sat Apr 05 16:48:27 2008 +0200 > @@ -2,6 +2,7 @@ > #define __DW_FLTKVIEWBASE_HH__ > > #include <fltk/Group.h> > +#include <fltk/Image.h> > #include <fltk/Scrollbar.h> > > #include "fltkcore.hh" > @@ -15,6 +16,7 @@ private: > private: > int bgColor; > core::Region drawRegion; > + static ::fltk::Image *backBuffer; > > void drawChildWidgets (); >

April 6, 2008

patch: FltkViewport::adjustScrollbarValues()

by Johannes.Hofmann＠gmx.de

On Sat, Apr 05, 2008 at 12:55:00PM -0400, Jorge Arellano Cid wrote: > On Fri, Apr 04, 2008 at 06:58:40PM +0000, corvid wrote: > > You know how the edges of pages like to hide under the scrollbars? > > Yes, I do! > > > This makes them stop doing that. > > Committed. Great, nice fix! Johannes

April 5, 2008

[BUG]: anchors break when a page includes '<meta http-equiv="Content-Type" ...'

by jcid＠dillo.org

On Sat, Apr 05, 2008 at 11:58:45AM +0200, Johannes Hofmann wrote: > > [...] > > jcid@d620:~/C/Dillo/d2/dillo2-cur/src$ ./dillo-fltk /tmp/ > > dillo_dns_init: Here we go! (threaded) > > Enabling cookies as from cookiesrc... > > Nav_open_url: new url='file:/tmp/' > > Nav_open_url: old_url='' idx=-1 > > [file dpi]: dpip_tag={<cmd='open_url' url='file:/tmp/' '>} > > adjustScrollPos: scrollX=0 scrollY=-155 > > ^^^^--- here! > > > > Note: my /tmp is shorter than the viewport. > > > > Please give it a look. > > Funny, I had a similar effect in fltkviewport.cc some time ago. The > problem becomes more obvious, when adding another printf at the > beginning of adjustScrollPos (). Then we get: > > Nav_open_url: new url='file:/tmp/hu' > Nav_open_url: old_url='' idx=-1 > [file dpi]: dpip_tag={<cmd='open_url' url='file:/tmp/hu' '>} > adjustScrollPos: scrollX=0 scrollY=0 > adjustScrollPos: scrollX=0 scrollY=-376 > > So its adjustScrollPos() itself that makes scrollY negative! > The fix is to do the checks the other way round and not to use > "else if" (see attached patch). Good it wasn't a complex side effect. Committed. > I noticed one regression with the new anchor code: > The "Continue Reading Older Posts" link on www.boingboing.net > (e.g. http://www.boingboing.net/2008/04/03/#entry-44240) no longer > scrolls to the right position. I'll check it. > PS: I hope the patch format is ok like this. I'm using mercurial and > the builtin diff may behave a bit differently than standard diff. I like "diff -pru", because of the context, and the explicit stating of the function where the changes happen. Mercurial seems to provide both (as in your attached patch). > Do you prefer inline or attached patches? Attached. -- Cheers Jorge.-

April 5, 2008

patch: FltkViewport::adjustScrollbarValues()

by jcid＠dillo.org

On Fri, Apr 04, 2008 at 06:58:40PM +0000, corvid wrote: > You know how the edges of pages like to hide under the scrollbars? Yes, I do! > This makes them stop doing that. Committed. -- Cheers Jorge.-

April 5, 2008

Good news from fltk!

by Johannes.Hofmann＠gmx.de

Hi, I stumbled across this post on fltk.bugs: http://fltk.org/newsgroups.php?s5281+gfltk.bugs+v5290+T0 This is great for us, as it allows us to implement efficient double buffering without X11 specific hacks. Attached is a patch that enables double buffering for partial screen redraws (this is where the flickering is most annoying). You will need at least version r6079 of fltk2 to test. Cheers, Johannes

April 5, 2008

[BUG]: anchors break when a page includes '<meta http-equiv="Content-Type" ...'

by Johannes.Hofmann＠gmx.de

Hi Jorge, On Thu, Apr 03, 2008 at 06:16:03PM -0400, Jorge Arellano Cid wrote: > Hi, > > On Fri, Mar 28, 2008 at 05:08:01PM +0000, Jeremy Henty wrote: > > On Fri, Mar 28, 2008 at 09:57:10AM -0400, Jorge Arellano Cid wrote: > > > > > > On Wed, Mar 26, 2008 at 10:23:48PM +0000, Jeremy Henty wrote: > > > > > > > > If the page contains a meta tag that changes the Content-Type then > > > > Dillo re-opens the URL. This calls Layout::removeWidget(), which > > > > in turn calls Layout::setAnchor(NULL). Ouch! If the URL > > > > contained a fragment then the anchor is lost. Worse, the anchor > > > > doesn't get restored because the URL is flagged as "just > > > > reloading" before it is reopened. The result is that the anchor > > > > mysteriously stops working. > > > > > > > > Commenting out the call to Layout::setAnchor(NULL) in > > > > Layout::removeWidget() fixes the bug and doesn't seem to break > > > > anything else, but I'm not convinced it's the right thing to do. > > > > > > In fact, repush is an after thought, and some parts of nav also > > > need some re-work. The API, IIRC pushing #fragment URLs to the stack > > > has some anomalies too. It'd be good to find/design a single path to > > > handle it (experience shows multiple paths is asking for trouble :). > > > I've looked at it and it seems like deep analysis is required. > > > > Thanks for explaining this. I'll keep running with > > Layout::setAnchor(NULL) commented out for the moment and see how it > > goes. (Still no breakage yet.) I hope to be able to spend time > > looking at this further. If I have any thoughts about a good way to > > do all this I will post them. Unfortunately I have had to devote more > > time to earning a living recently (sigh) so I've spent less time > > looking at Dillo code than I would have liked. > > After a long review of the code, I committed a patch that not only > fixes things in nav.c but in other places which were also involved > in the SEGFAULTs we were getting. Please test it Jeremy. > > Johannes, there's a patch for dw2 too. It involves minor initialization > of scrollX and scrollY, and a printf. If I open dillo with /tmp, a wild > value is got for scrollY: The added initialization is good I think. > > jcid@d620:~/C/Dillo/d2/dillo2-cur/src$ ./dillo-fltk /tmp/ > dillo_dns_init: Here we go! (threaded) > Enabling cookies as from cookiesrc... > Nav_open_url: new url='file:/tmp/' > Nav_open_url: old_url='' idx=-1 > [file dpi]: dpip_tag={<cmd='open_url' url='file:/tmp/' '>} > adjustScrollPos: scrollX=0 scrollY=-155 > ^^^^--- here! > > Note: my /tmp is shorter than the viewport. > > Please give it a look. Funny, I had a similar effect in fltkviewport.cc some time ago. The problem becomes more obvious, when adding another printf at the beginning of adjustScrollPos (). Then we get: Nav_open_url: new url='file:/tmp/hu' Nav_open_url: old_url='' idx=-1 [file dpi]: dpip_tag={<cmd='open_url' url='file:/tmp/hu' '>} adjustScrollPos: scrollX=0 scrollY=0 adjustScrollPos: scrollX=0 scrollY=-376 So its adjustScrollPos() itself that makes scrollY negative! The fix is to do the checks the other way round and not to use "else if" (see attached patch). I noticed one regression with the new anchor code: The "Continue Reading Older Posts" link on www.boingboing.net (e.g. http://www.boingboing.net/2008/04/03/#entry-44240) no longer scrolls to the right position. Cheers, Johannes PS: I hope the patch format is ok like this. I'm using mercurial and the builtin diff may behave a bit differently than standard diff. Do you prefer inline or attached patches?

April 5, 2008

patch: FltkViewport::adjustScrollbarValues()

by corvid＠lavabit.com

You know how the edges of pages like to hide under the scrollbars? This makes them stop doing that.

April 4, 2008