- Dillo-dev - lists.mailman3.com

whitespace entities inside words

by Johannes.Hofmann＠gmx.de

On Sun, May 10, 2009 at 04:15:13PM -0400, Jorge Arellano Cid wrote: > On Sun, May 10, 2009 at 09:48:35PM +0200, Johannes Hofmann wrote: > > On Fri, May 08, 2009 at 07:35:34PM +0000, corvid wrote: > > > Jorge wrote: > > > > On Fri, May 08, 2009 at 03:53:03AM +0000, corvid wrote: > > > > > Is a whitespace entity inside a word supposed to be > > > > > treated like normal whitespace? > > > > > > > > > > I thought it would be, but first I saw that > > > > > adjacent   ... aren't collapsed, > > > > > then I realized that we are leaving the > > > > > space inside the word instead of calling > > > > > addSpace, and then I saw that PrevWasSPC > > > > > isn't set if the space was at the end of the > > > > > word. > > > > > > > > > > If it's just something that no one ever got > > > > > around to dealing with, I'll make a patch. > > > > > > > > That hits into a deeper problem: the treatment of > > > > whitespace is not well defined inside the textblock yet. > > > > The same problem arises when handling NBSP. There's no > > > > clear definition on how to handle/flag it. > > > > If you're willing to study the textblock word wrapping, > > > > and the whitespace problem, great! I think the next big > > > > step for dillo is to handle DIV as a floating word (so it > > > > can be rendered side by side, as intended), and AFAIS that > > > > needs changes in the textblock. > > > > This is a task for 2.2. > > > > > > I have some Textblock code around to fix "white-space: nowrap" > > > (which shows up a lot on wikipedia, where the category stuff > > > at the bottoms of pages is displayed badly) > > > that I've been saving for discussion after this release. > > > I have absolutely no clue about floating, though, and have > > > been hoping that Johannes will develop an interest in it :) > > > > > > > Actually I've also been waiting for someone else to step up :) > > Sebastian told me long ago it could take him a week or so to > add this in the textblock, and that he was going to do it. > Unofortunately, having no news of him, it looks like I'll have to > dive into it at some point in time. > > I remember doing this with table rendering algorithms, and it > took months, but the end result was worth the effort. If somebody > here wants to help along the way, it'd be more than welcomed. I guess there will be some trouble with the redraw optimization I added to Textblock some time ago. I'll be happy to help to make this work with floats. Cheers, Johannes

May 10, 2009

whitespace entities inside words

by jcid＠dillo.org

On Sun, May 10, 2009 at 09:48:35PM +0200, Johannes Hofmann wrote: > On Fri, May 08, 2009 at 07:35:34PM +0000, corvid wrote: > > Jorge wrote: > > > On Fri, May 08, 2009 at 03:53:03AM +0000, corvid wrote: > > > > Is a whitespace entity inside a word supposed to be > > > > treated like normal whitespace? > > > > > > > > I thought it would be, but first I saw that > > > > adjacent   ... aren't collapsed, > > > > then I realized that we are leaving the > > > > space inside the word instead of calling > > > > addSpace, and then I saw that PrevWasSPC > > > > isn't set if the space was at the end of the > > > > word. > > > > > > > > If it's just something that no one ever got > > > > around to dealing with, I'll make a patch. > > > > > > That hits into a deeper problem: the treatment of > > > whitespace is not well defined inside the textblock yet. > > > The same problem arises when handling NBSP. There's no > > > clear definition on how to handle/flag it. > > > If you're willing to study the textblock word wrapping, > > > and the whitespace problem, great! I think the next big > > > step for dillo is to handle DIV as a floating word (so it > > > can be rendered side by side, as intended), and AFAIS that > > > needs changes in the textblock. > > > This is a task for 2.2. > > > > I have some Textblock code around to fix "white-space: nowrap" > > (which shows up a lot on wikipedia, where the category stuff > > at the bottoms of pages is displayed badly) > > that I've been saving for discussion after this release. > > I have absolutely no clue about floating, though, and have > > been hoping that Johannes will develop an interest in it :) > > > > Actually I've also been waiting for someone else to step up :) Sebastian told me long ago it could take him a week or so to add this in the textblock, and that he was going to do it. Unofortunately, having no news of him, it looks like I'll have to dive into it at some point in time. I remember doing this with table rendering algorithms, and it took months, but the end result was worth the effort. If somebody here wants to help along the way, it'd be more than welcomed. -- Cheers Jorge.-

May 10, 2009

whitespace entities inside words

by Johannes.Hofmann＠gmx.de

On Fri, May 08, 2009 at 07:35:34PM +0000, corvid wrote: > Jorge wrote: > > On Fri, May 08, 2009 at 03:53:03AM +0000, corvid wrote: > > > Is a whitespace entity inside a word supposed to be > > > treated like normal whitespace? > > > > > > I thought it would be, but first I saw that > > > adjacent   ... aren't collapsed, > > > then I realized that we are leaving the > > > space inside the word instead of calling > > > addSpace, and then I saw that PrevWasSPC > > > isn't set if the space was at the end of the > > > word. > > > > > > If it's just something that no one ever got > > > around to dealing with, I'll make a patch. > > > > That hits into a deeper problem: the treatment of > > whitespace is not well defined inside the textblock yet. > > The same problem arises when handling NBSP. There's no > > clear definition on how to handle/flag it. > > If you're willing to study the textblock word wrapping, > > and the whitespace problem, great! I think the next big > > step for dillo is to handle DIV as a floating word (so it > > can be rendered side by side, as intended), and AFAIS that > > needs changes in the textblock. > > This is a task for 2.2. > > I have some Textblock code around to fix "white-space: nowrap" > (which shows up a lot on wikipedia, where the category stuff > at the bottoms of pages is displayed badly) > that I've been saving for discussion after this release. > I have absolutely no clue about floating, though, and have > been hoping that Johannes will develop an interest in it :) > Actually I've also been waiting for someone else to step up :)

May 10, 2009

Nav_repush() is hurting valgrind

by jcid＠dillo.org

On Sun, May 10, 2009 at 08:20:09AM +0100, Jeremy Henty wrote: > > Here are two more that show the same thing. With a vanilla dillo they > leak DilloImages. The imgref patch that I attached earlier fixes the > leaks. > > http://stephenlaw.blogspot.com/2009/05/careful-how-you-use-bogus.html > http://www.nybooks.com/articles/22694 > > I've attached the valgrind logs. The "__0.txt" versions are from the > vanilla repo. The others are after applying the imgref patch. No > other patches were applied. I see "bogus_log.txt" has a SIGSEGV. Does it mean you get a segfault exit with the patch applied on that page? (it doesn't happen here, and I've got no segfault in a long time). -- Cheers Jorge.-

May 10, 2009

Nav_repush() is hurting valgrind

by jcid＠dillo.org

On Sun, May 10, 2009 at 08:20:09AM +0100, Jeremy Henty wrote: > On Sat, May 09, 2009 at 12:19:17PM -0400, Jorge Arellano Cid wrote: > > > I've found this: > > > > ==9830== 1,420 (352 direct, 1,068 indirect) bytes in 11 blocks are definitely > > lost in loss record 153 of 192 > > ==9830== at 0x4025D2E: malloc (vg_replace_malloc.c:207) > > ==9830== by 0x8081270: dMalloc (dlib.c:36) > > ==9830== by 0x807E46A: a_Image_new (image.cc:40) > > ==9830== by 0x80703C5: a_Html_image_new(DilloHtml*, char const*, int, > > _DilloUrl*) (html.cc:2067) > > ==9830== by 0x8074F49: _ZL16Html_input_imageP9DilloHtmlPKci (form.cc:1905) > > ==9830== by 0x8078C5A: Html_tag_open_input(DilloHtml*, char const*, int) > > (form.cc:484) > > ==9830== by 0x8071F71: _ZL16Html_process_tagP9DilloHtmlPci (html.cc:3455) > > ==9830== by 0x80725E8: _ZL14Html_write_rawP9DilloHtmlPcii (html.cc:3756) > > ==9830== by 0x807276F: DilloHtml::write(char*, int, int) (html.cc:573) > > ==9830== by 0x8072802: _ZL13Html_callbackiP12_CacheClient (html.cc:3647) > > ==9830== by 0x8060BC7: Cache_process_queue (cache.c:1154) > > ==9830== by 0x80603A0: a_Cache_process_dbuf (cache.c:876) > > > > in www.lacuarta.cl > > Here are two more that show the same thing. With a vanilla dillo they > leak DilloImages. The imgref patch that I attached earlier fixes the > leaks. > > http://stephenlaw.blogspot.com/2009/05/careful-how-you-use-bogus.html > http://www.nybooks.com/articles/22694 > > I've attached the valgrind logs. The "__0.txt" versions are from the > vanilla repo. The others are after applying the imgref patch. No > other patches were applied. > > I think it's clear that image reference counting is currently broken > and the imgref patch, or something like it, is needed to fix it. Patch reviewed and committed! Now, with this patch, is there any other Image-related problem detected by valgrind? AFAIS, it was made by Johannes, corvid and you? Just to give due credit in the ChangeLog. -- Cheers Jorge.-

May 10, 2009

Nav_repush() is hurting valgrind

by onepoint＠starurchin.org

On Sun, May 10, 2009 at 08:20:09AM +0100, Jeremy Henty wrote: > Here are two more that show the same thing. With a vanilla dillo > they leak DilloImages. The imgref patch that I attached earlier > fixes the leaks. > > http://stephenlaw.blogspot.com/2009/05/careful-how-you-use-bogus.html > http://www.nybooks.com/articles/22694 > > I've attached the valgrind logs. The "__0.txt" versions are from > the vanilla repo. The others are after applying the imgref patch. > No other patches were applied. Oops, I screwed up and overwrote one of the logs before attaching it. Ignore the one that reports a segfault. The others are OK: it's definitely the case that the vanilla Dillo leaks DilloImages on these URLs and that the imgref patch fixes the leak. Regards, Jeremy Henty

May 10, 2009

Nav_repush() is hurting valgrind

by onepoint＠starurchin.org

On Sat, May 09, 2009 at 12:19:17PM -0400, Jorge Arellano Cid wrote: > I've found this: > > ==9830== 1,420 (352 direct, 1,068 indirect) bytes in 11 blocks are definitely > lost in loss record 153 of 192 > ==9830== at 0x4025D2E: malloc (vg_replace_malloc.c:207) > ==9830== by 0x8081270: dMalloc (dlib.c:36) > ==9830== by 0x807E46A: a_Image_new (image.cc:40) > ==9830== by 0x80703C5: a_Html_image_new(DilloHtml*, char const*, int, > _DilloUrl*) (html.cc:2067) > ==9830== by 0x8074F49: _ZL16Html_input_imageP9DilloHtmlPKci (form.cc:1905) > ==9830== by 0x8078C5A: Html_tag_open_input(DilloHtml*, char const*, int) > (form.cc:484) > ==9830== by 0x8071F71: _ZL16Html_process_tagP9DilloHtmlPci (html.cc:3455) > ==9830== by 0x80725E8: _ZL14Html_write_rawP9DilloHtmlPcii (html.cc:3756) > ==9830== by 0x807276F: DilloHtml::write(char*, int, int) (html.cc:573) > ==9830== by 0x8072802: _ZL13Html_callbackiP12_CacheClient (html.cc:3647) > ==9830== by 0x8060BC7: Cache_process_queue (cache.c:1154) > ==9830== by 0x80603A0: a_Cache_process_dbuf (cache.c:876) > > in www.lacuarta.cl Here are two more that show the same thing. With a vanilla dillo they leak DilloImages. The imgref patch that I attached earlier fixes the leaks. http://stephenlaw.blogspot.com/2009/05/careful-how-you-use-bogus.html http://www.nybooks.com/articles/22694 I've attached the valgrind logs. The "__0.txt" versions are from the vanilla repo. The others are after applying the imgref patch. No other patches were applied. I think it's clear that image reference counting is currently broken and the imgref patch, or something like it, is needed to fix it. Regards, Jeremy Henty

May 10, 2009

Nav_repush() is hurting valgrind

by jcid＠dillo.org

On Sat, May 09, 2009 at 08:16:05PM +0100, Jeremy Henty wrote: > On Sat, May 09, 2009 at 12:19:17PM -0400, Jorge Arellano Cid wrote: > > > I've found this: > > > > ==9830== 1,420 (352 direct, 1,068 indirect) bytes in 11 blocks are definitely > > lost in loss record 153 of 192 > > [...] > > in www.lacuarta.cl > > I can reproduce this with a vanilla Dillo, but the leak disappears if > I apply the imgref patch that was suggested to me a while back. > (Patch attached.) Good! Image reference counting looks fishy inside the code. I'll review the patch. > > Please check whether the reported errors also happen in the > > vanilla repo. > > OK, I'll back out the image patches and see what the logs show then. > (It may take a few days before I can be sure if it's made a > difference.) I'd still suggest applying the imgref patch to the > mainline as it fixes your leak above. It looks as though DilloImages > aren't getting unrefed enough, and the patch fixes that. I still don't know what kind of valgrind problems are happening in vanilla dillo. This is useful information, but getting logs of a patched dillo is certainly confusing. -- Cheers Jorge.-

May 9, 2009

Nav_repush() is hurting valgrind

by onepoint＠starurchin.org

On Sat, May 09, 2009 at 12:19:17PM -0400, Jorge Arellano Cid wrote: > I've found this: > > ==9830== 1,420 (352 direct, 1,068 indirect) bytes in 11 blocks are definitely > lost in loss record 153 of 192 > [...] > in www.lacuarta.cl I can reproduce this with a vanilla Dillo, but the leak disappears if I apply the imgref patch that was suggested to me a while back. (Patch attached.) > Please check whether the reported errors also happen in the > vanilla repo. OK, I'll back out the image patches and see what the logs show then. (It may take a few days before I can be sure if it's made a difference.) I'd still suggest applying the imgref patch to the mainline as it fixes your leak above. It looks as though DilloImages aren't getting unrefed enough, and the patch fixes that. Regards, Jeremy Henty

May 9, 2009

Nav_repush() is hurting valgrind

by jcid＠dillo.org

On Sat, May 09, 2009 at 04:13:08PM +0100, Jeremy Henty wrote: > On Sat, May 09, 2009 at 10:00:55AM -0400, Jorge Arellano Cid wrote: > > On Sat, May 09, 2009 at 10:27:34AM +0100, Jeremy Henty wrote: > > > > > > The valgrind logs[1] show a recent burst of errors from calls to > > > Nav_repush(). > > > > Oh, here I can't reproduce that. I've tried with some sites and > > the valgrind log here is clean (a bit better than it used to be). > > Do you have an example URL or test case? > > Not yet. I'll see what I can dig out of the logs. I've found this: ==9830== 1,420 (352 direct, 1,068 indirect) bytes in 11 blocks are definitely lost in loss record 153 of 192 ==9830== at 0x4025D2E: malloc (vg_replace_malloc.c:207) ==9830== by 0x8081270: dMalloc (dlib.c:36) ==9830== by 0x807E46A: a_Image_new (image.cc:40) ==9830== by 0x80703C5: a_Html_image_new(DilloHtml*, char const*, int, _DilloUrl*) (html.cc:2067) ==9830== by 0x8074F49: _ZL16Html_input_imageP9DilloHtmlPKci (form.cc:1905) ==9830== by 0x8078C5A: Html_tag_open_input(DilloHtml*, char const*, int) (form.cc:484) ==9830== by 0x8071F71: _ZL16Html_process_tagP9DilloHtmlPci (html.cc:3455) ==9830== by 0x80725E8: _ZL14Html_write_rawP9DilloHtmlPcii (html.cc:3756) ==9830== by 0x807276F: DilloHtml::write(char*, int, int) (html.cc:573) ==9830== by 0x8072802: _ZL13Html_callbackiP12_CacheClient (html.cc:3647) ==9830== by 0x8060BC7: Cache_process_queue (cache.c:1154) ==9830== by 0x80603A0: a_Cache_process_dbuf (cache.c:876) in www.lacuarta.cl > > BTW, I'm talking about the vanilla repository. > > Are you testing with a patched repo? > > Yes. If it continues I'll back out any relevant patches and see if it > changes anything. I've added a list of my local patches to the > valgrind log page so that developers can see what I've done. > > http://starurchin.org/dillo/valgrind.html > > The only two that could (I think) possibly be relevant are: > > http://starurchin.org/dillo/patch/img_ref.diff.txt > http://starurchin.org/dillo/patch/make-a_Cache_stop_client-close-client.txt > > The first patch was suggested to me by the list when I first reported > the image access problems, the second is my attempt to fix it. I'd > really appreciate any comments on whether they are doing things the > right way. Now I'm working on this, comments should follow along the way. Please check whether the reported errors also happen in the vanilla repo. -- Cheers Jorge.-

May 9, 2009