- Dillo-dev - lists.mailman3.com

patch: clean up a_Html_add_image

by jcid＠dillo.org

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. It looks like an improvement. Committed. -- Cheers Jorge.-

Feb. 8, 2009

patch: <s> line appearance

by jcid＠dillo.org

On Sat, Feb 07, 2009 at 03:57:13PM +0000, corvid wrote: > 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. BTW, the new patch was the committed one. -- Cheers Jorge.-

Feb. 8, 2009

patch: <s> line appearance

by jcid＠dillo.org

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. > > I'm attaching an example html file as well. Committed. -- Cheers Jorge.-

Feb. 8, 2009

name/id bug

by jcid＠dillo.org

On Fri, Feb 06, 2009 at 05:38:44PM +0100, Hofmann Johannes wrote: > 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. You're right. -- Cheers Jorge.-

Feb. 8, 2009

[patch] Search Previous

by corvid＠lavabit.com

Nice! I notice that if I search backward for a space, it won't loop back to the end after reaching the top.

Feb. 8, 2009

CSS update committed

by Johannes.Hofmann＠gmx.de

Hi, I just committed a major update of the CSS code. CSS values now have a type and the CSS parser can accept multiple types for one property. One example is font-size, which can either be a length or an enum (xx-small, x-small, ...). This fixes the font size issue on wikipedia.org. Cheers, Johannes

Feb. 8, 2009

[patch] Search Previous

by jcid＠dillo.org

On Thu, Feb 05, 2009 at 10:04:32PM +0000, Jo?o Ricardo Louren?o wrote: > Qui, 2009-02-05 ?s 20:56 +0000, Jo?o Ricardo Louren?o escreveu: > > Qua, 2009-02-04 ?s 10:35 -0300, Jorge Arellano Cid escreveu: > > > On Wed, Feb 04, 2009 at 08:51:26AM +0100, Hofmann Johannes wrote: > > > > On Tue, Feb 03, 2009 at 11:54:56PM +0000, Jo?o Ricardo Louren?o wrote: > > > > > Ter, 2009-02-03 ?s 21:39 +0100, Hofmann Johannes escreveu: > > > > > > Hello, > > > > > > > > > > > > On Tue, Feb 03, 2009 at 07:48:18PM +0000, Jo?o Ricardo Louren?o wrote: > > > > > > > Greetings. > > > > > > > > > > > > > > I 'spotted' a TODO on searching 'backwards' and decided to have a go at > > > > > > > it. > > > > > > > > > > > > Great. > > > > > > > > > > > > > > > > > > > > Basically, I implemented the Search Previous feature. > > > > > > > > > > > > > > I must say I have difficulties understanding the code behind > > > > > > > FindtextState::search0(), so I did not base my code on it at all. Sure > > > > > > > it could possibly be optimized, but it seems to do the job for now, I > > > > > > > haven't found any bugs, if you find any, please do say. > > > > > > > > > > > > FindtextState implements the Knuth-Morris-Pratt algorithm > > > > > > http://en.wikipedia.org/wiki/Knuth%E2%80%93Morris%E2%80%93Pratt_algorithm > > > > > > for efficient searching. It is pretty clever but a bit tricky. > > > > > > I see some issues in your implementation. E.g. try to search for > > > > > > 'ab' in a string 'abababa'. > > > > > > Even more tricky is searching for ababb in abababb, i.e. the prefix > > > > > > matches, but once it no longer does you would need to backtrack. > > > > > > The easiest but inefficient way to implement searching > > > > > > is doing a strcmp() at every text position. The Knuth-Morris-Pratt > > > > > > algorithm avoids that. > > > > > > I think it would be best to adapt search0 somehow to allow backward > > > > > > searching - it's more or less the same thing as forward searching > > > > > > anyway. Some ugly edge cases when switching between forward and > > > > > > backward maybe :-) > > > > > > > > > > > > Thanks for working on that, > > > > > > Johannes > > > > > > > > > > > > > > > > > > > > Additionally, I did not set any hotkeys for the search-previous action, > > > > > > > maybe something like SHIFT+RETURN, since RETURN handles searching > > > > > > > forwards? > > > > > > > > > > > > > > Also, I did not add any tests, and I only altered a line to make them > > > > > > > compilable, but I might add some tests soon. > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Jo?o > > > > > > > > > > Thanks for your answer. > > > > > > > > > > OK, I am still trying to 'go deep' with the KMP algorithm, but I have > > > > > got (provisory) working search0Backwards() replacement. How efficient do > > > > > you think it is to handle the word by reversing it? Rephrasing that, do > > > > > you think there is a 'really' better way of searching backwards, than > > > > > reversing the word and searching backwards? As far as code-changes it > > > > > takes very few lines and I have a working example of it. However, the > > > > > extra reversing work might hit us hard. > > > > > > > > Reversing the word (needle) we are searching for is no problem. It's supposed > > > > to be rather small and we only need to reverse it once. > > > > The point is to avoid the backtracking. So that every letter in the > > > > text (haystack) is only visited once. > > > > > > If reversing the needle is all it takes, go ahead. > > > If the code is simple and it takes twice the time a forward > > > search, go ahead too. Backward search is much less used than > > > forward search and it's better to have it than not, and unless > > > the delay is annoying there's no compelling need to optimize. > > > > > > > > > > Thanks for the answer. > > > > OK, I think I worked it out, but the string is being reversed everytime > > we search backwards, to maintain compatibility with forward searching. > > Of course an alternative is to store it as private member, what do you > > think? Not necessary > > I couldn't find any function for reversing a C-String in dillo's code, > > so I put my own as a static member in FindtextState, should I move it > > elsewhere? Let's leave it there until needed. > > I tested multiple cases and it seems (to me) that this one is bugless, > > maybe you'll find a nasty/dirty bug I caused, but time will tell :) Now the "real world" test begins. > Scratch that, my bad, that patch contained various bugs. The first kind > of bug related to a typo which I now fixed. The second kind because of > me interpreting code the wrong way and the third one because of the > particular case of searching for things either at the beggining or at > the end of the page. I think I fixed all of them and have an updated > patch, excuse me for my previous mistakes... Committed! It has some changes: - Mainly whitespace (i.e. indentation). Please review them. - Added Shift+Enter as keyboard shortcut. - Simpler implementation of rev. It feels good! -- Cheers Jorge.-

Feb. 8, 2009

Rendering XHTML 1.1

by jcid＠dillo.org

On Mon, Feb 02, 2009 at 10:07:44AM +0100, Hole.destructor(a)gmx.de wrote: > Hello Dillo developers, > > since you are planing to release Dillo-2.1 in about a month, I am > wondering whether there are plans to support XHTML 1.1, XHTML 1.1 is possible now. It's disabled because we are not making a formal validation (though Firefox doesn't do it either). > HTML 5 and XHTML > 2 web pages in the near future? Not in the near future. Unless new developers, knowledgeable in that area, join us. -- Cheers Jorge.-

Feb. 8, 2009

Close-tab Button

by jorl17.8＠gmail.com

S?b, 2009-02-07 ?s 23:38 -0300, Jorge Arellano Cid escreveu: > On Sat, Feb 07, 2009 at 09:57:52PM +0000, Jo?o Ricardo Louren?o wrote: > > S?b, 2009-02-07 ?s 18:33 -0300, Jorge Arellano Cid escreveu: > > > On Fri, Feb 06, 2009 at 05:56:28PM -0300, Jorge Arellano Cid wrote: > > > > 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. > > > > > > Well, the good part is that it works and can be polished a bit. > > > The problem is side effects (e.g. redraw storms while moving the mouse > > > over the panel buttons, and who knows what else?). > > > > > > I tried a different approach and it almost worked OK, but after > > > resizing the window, the redraw storms are on the whole window. > > > > > > Frustrating... > > > > > > It looks like drawing the button in the pager could work. I'll > > > try to write some code this way. > > > > > > > Thanks. Redraw storms are visible when running it under valgrind. Also, > > parts of the code there were/are unnecessary. For instance, we could > > send the redraw() message only when adding a new tab and not > > continuously, but the redraw storms continue either way... > > > > Adding it to the pager seems the way to go there, but will you add it as > > a button, or a custom drawn widget? > > > > If you need any help, do say, I'll try to do my best, > > Attached goes a patch that behaves nicely. > Please polish it to a final state. It is commented. > > The button drawing code lacks the "X", the callback is not hooked, > and there's no tooltip, but it doesn't trigger redraw storms, > and feels right. > > Good luck! OK, I've already implemented update_positions() to prevent tabs from going over the button (grabbed it from FLTK and altered it). I don't think there is the need for a callback function, since it only calls a simple line. I've noticed that we have no checking if the button is visible or not, so it is working even though there are no tabs, I'll add a check for that in event() rather than in other places. Thanks, Jo?o

Feb. 8, 2009

patch: clean up a_Html_add_image

by onepoint＠starurchin.org

On Sat, Feb 07, 2009 at 11:59:03PM +0000, corvid wrote: > Jeremy wrote: > > On Sat, Feb 07, 2009 at 08:20:18PM +0000, corvid wrote: > > > > > > In principle it might be dangerous in some way for the callers > > > to wait to add widgets until after Html_load_image() is called, > > > > If we want the a_Html_* API to guard against that then that is > > another reason for changing it, since the current API allows you > > to create the image and not add it. [...] > > I'm not sure the "danger" is really worth worrying about; it's just > something that crossed my mind... Sure, but even if there's no real danger it's nice to have an API that expresses how the code is meant to work. Regards, Jeremy Henty

Feb. 8, 2009