February 2009 - Dillo-dev - lists.mailman3.com

patch: clean up a_Html_add_image

by Johannes.Hofmann＠gmx.de

On Sat, Feb 07, 2009 at 05:57:21PM +0000, Jeremy Henty wrote: > > There is IMHO a nasty code smell around a_Html_add_new_image(): it > takes a boolean parameter that determines whether or not it actually > adds the image. It's only called from two other places, once with the > parameter set to false and once with it set to true. > > This patch cleans things up by renaming a_Html_add_new_image() to > a_Html_image_new(), removing the boolean parameter and making the > caller add the image (or not) as appropriate. > > Perhaps it is a judgement call as to whether this is an improvement, > but I feel that it is. Please comment. did you forget to attach the patch? Cheers, Johannes

Feb. 7, 2009

patch: clean up a_Html_add_image

by onepoint＠starurchin.org

There is IMHO a nasty code smell around a_Html_add_new_image(): it takes a boolean parameter that determines whether or not it actually adds the image. It's only called from two other places, once with the parameter set to false and once with it set to true. This patch cleans things up by renaming a_Html_add_new_image() to a_Html_image_new(), removing the boolean parameter and making the caller add the image (or not) as appropriate. Perhaps it is a judgement call as to whether this is an improvement, but I feel that it is. Please comment. Regards, Jeremy Henty

Feb. 7, 2009

patch: <s> line appearance

by corvid＠lavabit.com

Johannes wrote: > On Sat, Feb 07, 2009 at 05:01:12AM +0000, corvid wrote: > > I was thinking that large text with a thin line through it > > looks odd. > > Turns out that there was also a problem with causing the > > <s> line to be too high. > > > > This looks about right to me with the few fonts I have around. > > Maybe one could determine the line width based on the xHeight member > of the font? Just an idea. Ah, good. 'diff' was sort of compensating for what sup and sub do to ascent and descent, but imperfectly -- as you probably noticed. Here's a new patch.

Feb. 7, 2009

patch: <s> line appearance

by Johannes.Hofmann＠gmx.de

On Sat, Feb 07, 2009 at 05:01:12AM +0000, corvid wrote: > I was thinking that large text with a thin line through it > looks odd. > Turns out that there was also a problem with causing the > <s> line to be too high. > > This looks about right to me with the few fonts I have around. Maybe one could determine the line width based on the xHeight member of the font? Just an idea. Cheers, Johannes

Feb. 7, 2009

patch: <s> line appearance

by corvid＠lavabit.com

I was thinking that large text with a thin line through it looks odd. Turns out that there was also a problem with causing the <s> line to be too high. This looks about right to me with the few fonts I have around. I'm attaching an example html file as well. PS Might be good to go through that area of the code one day to see what opportunities there are to make it clearer...

Feb. 7, 2009

Close-tab Button

by jcid＠dillo.org

On Mon, Feb 02, 2009 at 12:37:18AM +0000, Jo?o Ricardo Louren?o wrote: > Dom, 2009-02-01 ?s 14:51 -0300, Jorge Arellano Cid escreveu: > > On Sun, Feb 01, 2009 at 12:06:49AM +0000, Jo?o Ricardo Louren?o wrote: > > > Greetings to all. > > > > > > As a simple introduction may I say I thank you all for your work on > > > Dillo. > > > > > > Currently, I am trying to implement the Close-Tab button. I've been > > > discussing it with Jorge Arellano Cid and he suggested I 'brought-it' to > > > dillo-dev. > > > > > > FLTK Tabs don't support a close-tab button, as many of you have seen, > > > and the idea was to have a button at the end of the tabs list. > > > > > > For that, I created an fltk::Button widget in CustTabGroup. I position > > > this widget at the end of the tabs list and, then, I add() it to the > > > CustTabGroup's parent() (UI). > > > > > > After that, I set the UI's resizer() to be an InvisibleBox positioned in > > > such a way that it doesn't resize the button itself, but only moves it. > > > > > > Refreshing the button (when changing/adding/closing tabs) is done in the > > > handle() method of the CustTabGroup. > > > > > > Just a second ago I solved one of the problems, which was the fact that > > > tab_height() returned 0 in the constructor and resize() didn't work. I > > > worked around that by constructing the button only in event() if > > > tab_height()>0 and it hasn't been constructed yet. This way we can use > > > the correct height for the button. > > > > > > It may not seem reliable to do so many things in handle(). Any > > > suggestions? > > > > We need to get something working, review it, and then we may > > ask in fltk.general whether it could have side effects. > > > > > It is functional, only you can say if it fully suits Dillo's needs > > > though. > > > > Please post the patch here so we can look at it. > > > > > Unfortunately, there are some problems: > > > > > > The first one is the most-relevant one. When the list of tabs grows it > > > can go over the button. I have been trying to fix this, but I can't seem > > > to do it. > > > A possible way could (maybe) be to 'trick' the pager into thinking it > > > has less space left to draw the tabs, and that could possibly be > > > achieved using update_positions(), although I am having some problems > > > with that. > > > > Or maybe just tweaking the value. After all it's a workaround. > > > > > The second problem is a minor one, but annoying. I believe when CSS > > > rendering is triggered, the render area flickers and that is normal. > > > However, the way I implemented this (probably because of it being in > > > handle()) makes the Button also flash. > > > > A side affect of being a child of UI. > > > > So, as requested, here is what I have got until now. There is a > particular line I think is not necessary, I commented that specifically. I'm reviewing it now. -- Cheers Jorge.-

Feb. 6, 2009

name/id bug

by Johannes.Hofmann＠gmx.de

On Fri, Feb 06, 2009 at 01:34:24PM -0300, Jorge Arellano Cid wrote: > On Fri, Feb 06, 2009 at 04:01:58PM +0100, Hofmann Johannes wrote: > > On Fri, Feb 06, 2009 at 11:59:01AM -0300, Jorge Arellano Cid wrote: > > > On Fri, Feb 06, 2009 at 12:38:20PM +0100, Hofmann Johannes wrote: > > > > On Fri, Feb 06, 2009 at 11:06:11AM +0100, Hofmann Johannes wrote: > > > > > On Fri, Feb 06, 2009 at 10:58:37AM +0100, Hofmann Johannes wrote: > > > > > > On Fri, Feb 06, 2009 at 06:22:54AM +0000, corvid wrote: > > > > > > > <a name="a"></a> > > > > > > > > > > > > > > > > > > > > > <a name="same" id="same"></a> > > > > > > > > > > > > > > gives > > > > > > > HTML warning: line 2, 'id' and 'name' attribute of <a> tag differ > > > > > > > HTML warning: line 4, Anchor names must be unique within the document > > > > > > > > > > > > > > Dillo would seem to be unhappy now that the id attr is checked before > > > > > > > open_a checks the name attr -- but then it looks like the move was > > > > > > > made in October (a88b4a31cb7a) and I have a dillo around from > > > > > > > roughly Dec. 22 that does not have a problem with this. > > > > > > > > > > > > Yes, the problem was caused by moving the check. > > > > > > > > > > > > > > > > > > > > In any case, I don't know whether there are styleEngine issues with > > > > > > > just sticking the id check back after the open. > > > > > > > > > > > > We need to parse the "id" attribute before the open, as it's used to > > > > > > determine the style used in open*. > > > > > > > > > > > > But I don't understand the reason for the name/id check in the first > > > > > > place: > > > > > > > > > > > > <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > > > > > > <html> > > > > > > <head> <title>foo bar</title> </head> > > > > > > <body> > > > > > > <a name="foo" id="bar">foo bar</a> > > > > > > </body> > > > > > > </html> > > > > > > > > > > > > is correct HTML according to http://validator.w3.org/. > > > > > > I would propose to remove this check completely. Opinions? > > > > > > > > > > Ooops, stop! > > > > > I just found this: http://www.validome.org/lang/ge/errors/MISC/3005 > > > > > It seems that name and id must be equal and it's a bug in > > > > > http://validator.w3.org/ that it doesn't catch it. > > > > > I will try to fix the check in dillo. > > > > > > > > Please test/review current tip > > > > > > After a quick review, it seems to me that the proper solution > > > is to add a test in the open function for A, APPLET, FORM, FRAME, > > > IFRAME, IMG and MAP. Something like: > > > > > > if (id && nameVal && strcmp(nameVal, id)) > > > BUG_MSG("'id' and 'name' attribute of <%s> tag differ\n", tag_name); > > > if (nameVal) > > > Html_add_anchor(html, nameVal); > > > > If id and nameVal don't differ, we don't want to call > > Html_add_anchor(). It has already been called with the same value > > from Html_parse_common_attrs(). > > > > We could put the corresponding code that is currently in > > Html_tag_open_a() in a new function Html_handle_name_attr() and call > > that from all elements that allow a "name" attribute. > > Right, that's the idea. > Maybe: > > if (id && strcmp(nameVal, id) == 0) { > BUG_MSG("'id' and 'name' attribute of <a> tag differ\n"); > } else { > Html_add_anchor(html, nameVal); > } That's not quite it. I think the original test, that I now moved to Html_tag_open_a() is ok: if (!id || strcmp(nameVal, id)) { if (id) BUG_MSG("'id' and 'name' attribute of <a> tag differ\n"); Html_add_anchor(html, nameVal); } I.e. do something if id is not set or id and name differ. Then if they differ output a warning. Add an anchor for the new string. Cheers, Johannes

Feb. 6, 2009

name/id bug

by jcid＠dillo.org

On Fri, Feb 06, 2009 at 04:01:58PM +0100, Hofmann Johannes wrote: > On Fri, Feb 06, 2009 at 11:59:01AM -0300, Jorge Arellano Cid wrote: > > On Fri, Feb 06, 2009 at 12:38:20PM +0100, Hofmann Johannes wrote: > > > On Fri, Feb 06, 2009 at 11:06:11AM +0100, Hofmann Johannes wrote: > > > > On Fri, Feb 06, 2009 at 10:58:37AM +0100, Hofmann Johannes wrote: > > > > > On Fri, Feb 06, 2009 at 06:22:54AM +0000, corvid wrote: > > > > > > <a name="a"></a> > > > > > > > > > > > > > > > > > > <a name="same" id="same"></a> > > > > > > > > > > > > gives > > > > > > HTML warning: line 2, 'id' and 'name' attribute of <a> tag differ > > > > > > HTML warning: line 4, Anchor names must be unique within the document > > > > > > > > > > > > Dillo would seem to be unhappy now that the id attr is checked before > > > > > > open_a checks the name attr -- but then it looks like the move was > > > > > > made in October (a88b4a31cb7a) and I have a dillo around from > > > > > > roughly Dec. 22 that does not have a problem with this. > > > > > > > > > > Yes, the problem was caused by moving the check. > > > > > > > > > > > > > > > > > In any case, I don't know whether there are styleEngine issues with > > > > > > just sticking the id check back after the open. > > > > > > > > > > We need to parse the "id" attribute before the open, as it's used to > > > > > determine the style used in open*. > > > > > > > > > > But I don't understand the reason for the name/id check in the first > > > > > place: > > > > > > > > > > <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > > > > > <html> > > > > > <head> <title>foo bar</title> </head> > > > > > <body> > > > > > <a name="foo" id="bar">foo bar</a> > > > > > </body> > > > > > </html> > > > > > > > > > > is correct HTML according to http://validator.w3.org/. > > > > > I would propose to remove this check completely. Opinions? > > > > > > > > Ooops, stop! > > > > I just found this: http://www.validome.org/lang/ge/errors/MISC/3005 > > > > It seems that name and id must be equal and it's a bug in > > > > http://validator.w3.org/ that it doesn't catch it. > > > > I will try to fix the check in dillo. > > > > > > Please test/review current tip > > > > After a quick review, it seems to me that the proper solution > > is to add a test in the open function for A, APPLET, FORM, FRAME, > > IFRAME, IMG and MAP. Something like: > > > > if (id && nameVal && strcmp(nameVal, id)) > > BUG_MSG("'id' and 'name' attribute of <%s> tag differ\n", tag_name); > > if (nameVal) > > Html_add_anchor(html, nameVal); > > If id and nameVal don't differ, we don't want to call > Html_add_anchor(). It has already been called with the same value > from Html_parse_common_attrs(). > > We could put the corresponding code that is currently in > Html_tag_open_a() in a new function Html_handle_name_attr() and call > that from all elements that allow a "name" attribute. BTW, http://www.validome.org/lang/ge/errors/MISC/3005 says that they must be equal for the above mentioned elements only. I don't know what's the rule for other elements with "name" attr. though. -- Cheers Jorge.-

Feb. 6, 2009

name/id bug

by jcid＠dillo.org

On Fri, Feb 06, 2009 at 04:01:58PM +0100, Hofmann Johannes wrote: > On Fri, Feb 06, 2009 at 11:59:01AM -0300, Jorge Arellano Cid wrote: > > On Fri, Feb 06, 2009 at 12:38:20PM +0100, Hofmann Johannes wrote: > > > On Fri, Feb 06, 2009 at 11:06:11AM +0100, Hofmann Johannes wrote: > > > > On Fri, Feb 06, 2009 at 10:58:37AM +0100, Hofmann Johannes wrote: > > > > > On Fri, Feb 06, 2009 at 06:22:54AM +0000, corvid wrote: > > > > > > <a name="a"></a> > > > > > > > > > > > > > > > > > > <a name="same" id="same"></a> > > > > > > > > > > > > gives > > > > > > HTML warning: line 2, 'id' and 'name' attribute of <a> tag differ > > > > > > HTML warning: line 4, Anchor names must be unique within the document > > > > > > > > > > > > Dillo would seem to be unhappy now that the id attr is checked before > > > > > > open_a checks the name attr -- but then it looks like the move was > > > > > > made in October (a88b4a31cb7a) and I have a dillo around from > > > > > > roughly Dec. 22 that does not have a problem with this. > > > > > > > > > > Yes, the problem was caused by moving the check. > > > > > > > > > > > > > > > > > In any case, I don't know whether there are styleEngine issues with > > > > > > just sticking the id check back after the open. > > > > > > > > > > We need to parse the "id" attribute before the open, as it's used to > > > > > determine the style used in open*. > > > > > > > > > > But I don't understand the reason for the name/id check in the first > > > > > place: > > > > > > > > > > <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > > > > > <html> > > > > > <head> <title>foo bar</title> </head> > > > > > <body> > > > > > <a name="foo" id="bar">foo bar</a> > > > > > </body> > > > > > </html> > > > > > > > > > > is correct HTML according to http://validator.w3.org/. > > > > > I would propose to remove this check completely. Opinions? > > > > > > > > Ooops, stop! > > > > I just found this: http://www.validome.org/lang/ge/errors/MISC/3005 > > > > It seems that name and id must be equal and it's a bug in > > > > http://validator.w3.org/ that it doesn't catch it. > > > > I will try to fix the check in dillo. > > > > > > Please test/review current tip > > > > After a quick review, it seems to me that the proper solution > > is to add a test in the open function for A, APPLET, FORM, FRAME, > > IFRAME, IMG and MAP. Something like: > > > > if (id && nameVal && strcmp(nameVal, id)) > > BUG_MSG("'id' and 'name' attribute of <%s> tag differ\n", tag_name); > > if (nameVal) > > Html_add_anchor(html, nameVal); > > If id and nameVal don't differ, we don't want to call > Html_add_anchor(). It has already been called with the same value > from Html_parse_common_attrs(). > > We could put the corresponding code that is currently in > Html_tag_open_a() in a new function Html_handle_name_attr() and call > that from all elements that allow a "name" attribute. Right, that's the idea. Maybe: if (id && strcmp(nameVal, id) == 0) { BUG_MSG("'id' and 'name' attribute of <a> tag differ\n"); } else { Html_add_anchor(html, nameVal); } -- Cheers Jorge.-

Feb. 6, 2009

[patch]: fix leak in a_Cookies_get_query()

by jcid＠dillo.org

On Thu, Feb 05, 2009 at 07:55:46PM +0000, Jeremy Henty wrote: > > This patch stops a_Cookies_get_query() leaking. Committed. -- Cheers Jorge.-

Feb. 6, 2009