- Dillo-dev - lists.mailman3.com

CSS leaks: WAS Re: [Dillo-dev] <ol>'s leak the labels of their items

by onepoint＠starurchin.org

On Mon, Jan 26, 2009 at 11:33:43PM +0100, Hofmann Johannes wrote: > There is at least one leak I know of. It's font-family names that > are parsed in CSS style data. Aha! I looked at the Valgrind log for that one but didn't know the cause. Valgrind identifies the dStrdup() in val->strVal = dStrdup(parser->tval); in Css_parse_value() as the leak. What's the fix? I thought the problem was that CssPropertyList::set() does not free any existing value but I'm not sure I understand how these classes are meant to manage their memory. Regards, Jeremy Henty

Jan. 27, 2009

<ol>'s leak the labels of their items

by Johannes.Hofmann＠gmx.de

On Mon, Jan 26, 2009 at 10:32:42PM +0000, Jeremy Henty wrote: > On Mon, Jan 26, 2009 at 11:03:20PM +0100, Hofmann Johannes wrote: > > On Mon, Jan 26, 2009 at 09:22:03PM +0000, Jeremy Henty wrote: > > > > > > Valgrind reports that <ol>s leak the labels of their items: each > > > of the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per > > > <li> etc. The leaked memory is created by the line > > > > > > list_item->initWithText (dStrdup(buf), wordStyle); > > > > Ah right. The strdup() can just be dropped. Allocation and > > deallocation of text is done in Textblock itself for quite some time > > now. Please check current tip. > > It's fixed! (/me deletes valgrind suppression file) good > > But..., valgrind still reports some "indirectly lost" blocks despite > reporting no "definitely lost" or "possibly lost" blocks. How is that > possible? I am suppressing some leaks that come from the depths of X > windows and other libraries, could that be the reason? (Of course > this may have nothing to do with Dillo.) There is at least one leak I know of. It's font-family names that are parsed in CSS style data. Regards, Johannes

Jan. 26, 2009

<ol>'s leak the labels of their items

by onepoint＠starurchin.org

On Mon, Jan 26, 2009 at 11:03:20PM +0100, Hofmann Johannes wrote: > On Mon, Jan 26, 2009 at 09:22:03PM +0000, Jeremy Henty wrote: > > > > Valgrind reports that <ol>s leak the labels of their items: each > > of the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per > > <li> etc. The leaked memory is created by the line > > > > list_item->initWithText (dStrdup(buf), wordStyle); > > Ah right. The strdup() can just be dropped. Allocation and > deallocation of text is done in Textblock itself for quite some time > now. Please check current tip. It's fixed! (/me deletes valgrind suppression file) But..., valgrind still reports some "indirectly lost" blocks despite reporting no "definitely lost" or "possibly lost" blocks. How is that possible? I am suppressing some leaks that come from the depths of X windows and other libraries, could that be the reason? (Of course this may have nothing to do with Dillo.) Regards, Jeremy Henty

Jan. 26, 2009

<ol>'s leak the labels of their items

by Johannes.Hofmann＠gmx.de

On Mon, Jan 26, 2009 at 09:22:03PM +0000, Jeremy Henty wrote: > > Valgrind reports that <ol>s leak the labels of their items: each of > the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per <li> etc. > The leaked memory is created by the line > > list_item->initWithText (dStrdup(buf), wordStyle); Ah right. The strdup() can just be dropped. Allocation and deallocation of text is done in Textblock itself for quite some time now. Please check current tip. Cheers, Johannes

Jan. 26, 2009

<ol>'s leak the labels of their items

by onepoint＠starurchin.org

Valgrind reports that <ol>s leak the labels of their items: each of the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per <li> etc. The leaked memory is created by the line list_item->initWithText (dStrdup(buf), wordStyle); in Html_tag_open_li (in src/html.cc). The dStrdup()ed memory is never freed. I think that the problem is that the TextBlock destructor only deletes the content of a word if the content type is WIDGET or ANCHOR, whereas it should also handle the case when the content type is TEXT. Perhaps initWithText() should copy the string argument itself with new[] so that ~TextBlock could safely delete[] it? The only two references to initWithText() are in html.cc and one of the widget tests, so this would be an easy change. Regards, Jeremy Henty

Jan. 26, 2009

Re: new font preferences

by Johannes.Hofmann＠gmx.de

On Mon, Jan 26, 2009 at 04:58:55PM -0300, Jorge Arellano Cid wrote: > On Mon, Jan 26, 2009 at 07:14:08PM +0100, Hofmann Johannes wrote: > > Hi Jorge, > > > > do you have an opinion on the (incompatible) changes to the font > > preferences as proposed in > > http://lists.auriga.wearlab.de/pipermail/dillo-dev/2009-January/005746.html > > > > I'd like to commit this if you agree. > > Oh yes, feel free to commit. Committed. > Just let dillo-dev know how to upgrade our custom dillorc! :-) > > At this point you're the most knowledgeable in font handling and > CSS, thus some basic explanation of settings/usage would be welcomed too. >From the checking comment: CSS 2.1 uses the following predefined font names: 'serif', 'sans-serif', 'cursive', 'fantasy', 'monospace'. Add preferences so that the user can set real font names for these. Basically the new "font_sans_serif" option is what "vw_fontame" used to be and "font_monospace" is equivalent to "fw_fontname". The other two options are new. http://www.w3.org/TR/CSS21/fonts.html#generic-font-families has some more information including font examples. Cheers, Johannes

Jan. 26, 2009

dillo CSS and charset

by jcid＠dillo.org

On Thu, Jan 22, 2009 at 06:53:46PM +0000, corvid wrote: > Johannes wrote: > > On Thu, Jan 22, 2009 at 05:57:42PM +0000, corvid wrote: > > > Johannes wrote: > > > > BTW. now you can set vw_fontname in dillorc again. This font will > > > > then be used when CSS specifies "sans" as font, e.g: > > > > vw_fontname="VL Gothic" > > > > > > If I put a serif font in vw_fontname and then use > > > sans in style.css for some things, it'll use > > > the serif font. > > > > True. We need to make new config options > > > > sans-serif_fontname > > serif_fontname > > cursive_fontname > > fantasy_fontname > > monospace_fontname > > > > maybe even dropping the "_fontname" suffix. > > > > But of course that would no longer be compatible with older > > versions of dillorc. > > I would add the new options and remove the old options. > CSS is big and certainly worth some incompatibility. I agree with corvid. And, dillo should be able to pick some sane defaults when we figure how to deal with them right through FLTK. > Related to this, I've been wondering whether we should have > a policy about the dillorc on the website -- whether it should > follow tip, or follow the most recent release, or always add > new options but never delete old options... The current practice is to make it compatible with the last release, and to update the one in the source tree for those using the repository. -- Cheers Jorge.-

Jan. 26, 2009

Character entity parsing regression.

by jcid＠dillo.org

On Mon, Jan 26, 2009 at 07:09:55AM +0000, corvid wrote: > Jorge wrote: > > There's a regression with the attached page. Squared boxes > > appear where nothing should be. I haven't dug since when. > > Attached. Looks like it dates to > changeset: 286:d16cec5095be > user: jcid > date: Fri Jun 20 19:10:04 2008 +0200 > summary: - Adapted the code to the new Textblock::addText() API. > Good! Committed I also committed another fix for an unrelated but similar problem. Use "hg log -v|less" for a test case. -- Cheers Jorge.-

Jan. 26, 2009

CSS parser leak

by onepoint＠starurchin.org

On Sun, Jan 25, 2009 at 06:07:58PM +0100, Hofmann Johannes wrote: > I decided to stick with SimpleVector, but allocate CssSimpleSelector > properly with new/delete. Please check current tip. This seems to have fixed all the big "definitely lost" errors, which is very good. I still see some small "possibly lost" errors. The valgrind docs advise that "possibly lost" really means "definitely lost" unless you are doing something funny. I can't tell whether that applies here! As I understand it, "possibly lost" means that there is still a pointer to the interior of the block. Most likely the original pointer to the block has been lost and it has therefore leaked, but it is possible that the program knows about the interior pointer and is using that to access the block. Valgrind log attached. Regards, Jeremy Henty

Jan. 26, 2009

switch to DILLO_HTML_PARSE_MODE_PRE depending on style

by Johannes.Hofmann＠gmx.de

On Sun, Jan 25, 2009 at 06:10:33PM -0300, Jorge Arellano Cid wrote: > On Thu, Jan 22, 2009 at 02:39:32PM +0100, Hofmann Johannes wrote: > > Hi, > > > > I'm experimenting with attached patch to make switching to > > DILLO_HTML_PARSE_MODE_PRE from CSS possible. > > Some online source browsers like cgit (http://hjemli.net/git/cgit/) > > use this to display sources or diffs. > > > > Please comment. > > Considering the current length of the Html_process_tag function, > and that the proposed patch may have secondary effects if a tag > with automatic close sets PRE, I'd suggest something along the > lines of the attached patch. If it passes your tests, feel free > to commit. Committed in two steps. Thanks, Johannes

Jan. 26, 2009