Frank,
> Jorge Arellano Cid wrote:
>
> >Hi Frank,
> >
> > After waiting for a week as you suggested, today I gave a
> >review to the new kbnav patch. It feels good!
> >
> > After some testing I came across a page that produces a
> >segfault. The attached patch is against today's CVS + a couple of
> >compiler-warning removals. The page is also attached. Beware that
> >the segfault is only caught when the new parser is used, and that
> >the page has bad-html. The case is interesting though, because a
> >weird widget tree is generated and iterators should be able to
> >handle/survive them!
> >
> > Please give it a look.
> >
> > Cheers
> > Jorge.-
> >
> I have taken a look at your patch-to-my-patch, but did not apply it to
> the version I will publish later today.
:-o
> I did do some manual
> reformatting to correct some tab-related problems
Good!
> (Emacs' use of tabs
> messes up things sometimes, and the 2-space indent it uses are not
> consistent with Dillo's conventions).
Thanks, we strive to be style consistent.
BTW, I didn't try to fix indentation in the patch, it
was just to do a test run.
> Some files seem to be missing from
> the patch you sent me, I guess you did not make any changes to those?
My mistake.
>
> I did not see anything in your patch which would have corrected any
> compiler warnings by that way, with a possible exception for the
> disputed URL_ILLEGAL_CHARS_ macro. In my patch I used
> URL_ILLEGAL_CHARS_(u) in html.c, while you use URL_ILLEGAL_CHARS(u). In
> the current patch I use URL_ILLEGAL_CHARS(u) as well. The end result
> should be the same however now that both versions exist. What compiler
> warnings did you get which were corrected by your modifications to my patch?
focus.c: In function `Focus_state_find_list':
focus.c:268: warning: `list' might be used uninitialized in this
function
focus.c: In function `Focus_state_find_link':
focus.c:465: warning: `list' might be used uninitialized in this
function
gcc version 3.2.3
>
> On that Tarot-page you added to the message, I can say that navigation
> is still quirky. But so is the page, it produces 9 warnings about
> misplaced </a> tags. If you look at the source, you'll notice that it
> misses several closing </A> tags (causing Dillo to allocate links all
> over the page). It also contains some links with <BR> in them, which
> used to crash patched Dillo because of the problem with
> a_Dw_page_construct_iterator (which used to always set the mask to
> DW_CONTENT_ALL, I changed this to a user-settable mask like all other
> iterators have). I guess you attached the page because of those crashes?
Yes, for you to have a way to reproduce it.
BTW, that is what I meant with: "page has bad-html".
> I will probably publish a new version of the patch (4) today, together
> with a new version of the tab/frame patch. That version should be more
> or less ready to commit, as no serious problems have cropped up for
> several days now. See the list for problems found and corrected
> (including the one causing the Tarot page to crash by the way - see my
> message dated Wed, 12 Nov 2003 11:00:58 +0100).
Oh, I didn't find anything about it at GMANE !?
Cheers
Jorge.-