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

[patch] Search Previous

by jorl17.8＠gmail.com

Dom, 2009-02-08 ?s 20:59 +0000, corvid escreveu: > Nice! > > I notice that if I search backward for a space, > it won't loop back to the end after reaching the top. > > > Thanks for reporting that. I've been digging through the code, seeking for an explanation, but can't seem to find it. Any ideas? It's odd that it happens only with a space, and I've been checking and it *seems* that the code finds the character, what can be wrong? Anyway, I'll keep digging... Jo?o

Feb. 9, 2009

CSS update committed

by Johannes.Hofmann＠gmx.de

On Sun, Feb 08, 2009 at 07:38:53PM -0300, Jorge Arellano Cid wrote: > On Sun, Feb 08, 2009 at 09:18:04PM +0100, Hofmann Johannes wrote: > > 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. > > I just saw the big commit! > > It'd be very good if you provide us with examples of what > is changing (just as you did, just a bit more for non CSS-savvy > guys like me ;). The visible change is that "font-size: normal", "font-size: smaller", or "font-size: xx-large", etc. should work now as expected. In addition font-weight handling has been moved to the new scheme. The commit also provides the ground work to support the "inherit" special value in the future. Cheers, Johannes

Feb. 9, 2009

CSS update committed

by jcid＠dillo.org

On Sun, Feb 08, 2009 at 09:18:04PM +0100, Hofmann Johannes wrote: > 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. I just saw the big commit! It'd be very good if you provide us with examples of what is changing (just as you did, just a bit more for non CSS-savvy guys like me ;). -- Cheers Jorge.-

Feb. 8, 2009

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