- Dillo-dev - lists.mailman3.com

Re: Splitting html.cc

by jcid＠dillo.org

On Wed, Jun 04, 2008 at 03:38:38PM +0100, Jeremy Henty wrote: > > The first patch in this set creates stubs for the new files and adds > them to the build. Developers should run " make maintainer-clean ; > ./autogen.sh " after checking this out so that the new source file is > compiled. > > The other patches move some macros, typedefs, structs and class > definitions from html.cc into the new headers. Committed. > The next step is to move some of the function declarations across, but > that depends on deciding what to do with the function names. Should I > rename "Html_*" to "a_Html_*"? In other words, do functions in > form.hh count as part of the Html module API or part of its internals. > I am inclined to say they are internal and so not rename them. > Comments? I'd prefer to have C functions called from other sources as a_Html_*. -- Cheers Jorge.-

June 5, 2008

Re: Splitting html.cc

by onepoint＠starurchin.org

The first patch in this set creates stubs for the new files and adds them to the build. Developers should run " make maintainer-clean ; ./autogen.sh " after checking this out so that the new source file is compiled. The other patches move some macros, typedefs, structs and class definitions from html.cc into the new headers. The next step is to move some of the function declarations across, but that depends on deciding what to do with the function names. Should I rename "Html_*" to "a_Html_*"? In other words, do functions in form.hh count as part of the Html module API or part of its internals. I am inclined to say they are internal and so not rename them. Comments? Regards, Jeremy Henty

June 4, 2008

fltk2 snapshot update

by jcid＠dillo.org

On Mon, Jun 02, 2008 at 04:26:55PM +0200, Johannes Hofmann wrote: > Hi, > > at fltk.org there is a new fltk2 snaphot (r6132) available. It contains a > bugfix for the weird green artifacts in the FileChooser. I think we > should update the download instructions. > BTW, shouldn't we point users directly to fltk.org for downloading > fltk2? Otherwise people might think that dillo2 needs a > modified version of fltk2. Done. As the latest FLTK2 has worked OK for us the last months. -- Cheers Jorge.-

June 4, 2008

Re: Splitting html.cc

by jcid＠dillo.org

On Mon, Jun 02, 2008 at 09:23:44PM +0100, Jeremy Henty wrote: > > OK, I'll kick this off with a small patch that wraps the DilloHtmlForm > constructor and destructor in an API that doesn't need the class > declaration. Committed. > The next stage will be to create stubs for the new files and hook them > into the existing source and build system. That depends on deciding > what the new file names should be. My thoughts: > > The form implementation: form.cc or html_form.cc . form.cc > The form header: form.hh or html_form.hh . form.hh > The shared API for Html module internals: html_common.hh , > html_shared.hh , html_internal.hh or html_internals.hh . html_common.hh -- Cheers Jorge.-

June 4, 2008

Re: Splitting html.cc

by onepoint＠starurchin.org

D'oh, patch attached! Jeremy Henty

June 2, 2008

Re: Splitting html.cc

by onepoint＠starurchin.org

OK, I'll kick this off with a small patch that wraps the DilloHtmlForm constructor and destructor in an API that doesn't need the class declaration. The next stage will be to create stubs for the new files and hook them into the existing source and build system. That depends on deciding what the new file names should be. My thoughts: The form implementation: form.cc or html_form.cc . The form header: form.hh or html_form.hh . The shared API for Html module internals: html_common.hh , html_shared.hh , html_internal.hh or html_internals.hh . Please comment! Regards, Jeremy Henty

June 2, 2008

fltk2 snapshot update

by onepoint＠starurchin.org

On Mon, Jun 02, 2008 at 08:07:45PM +0100, Jeremy Henty wrote: > On Mon, Jun 02, 2008 at 08:48:57PM +0200, Johannes Hofmann wrote: > > > > Please let me know, if that problem still exists on your test > > page. > > Unfortunately the bug had disappeared again just before I upgraded > FLTK so I can't tell if it made any difference. And after a small refactoring it's back (albeit without the unpleasant green, so I guess that's something)! When I've cleared my other ToDos I'll valgrind it. Regards, Jeremy Henty

June 2, 2008

fltk2 snapshot update

by onepoint＠starurchin.org

On Mon, Jun 02, 2008 at 08:48:57PM +0200, Johannes Hofmann wrote: > On Mon, Jun 02, 2008 at 05:24:57PM +0100, Jeremy Henty wrote: > > On Mon, Jun 02, 2008 at 04:26:55PM +0200, Johannes Hofmann wrote: > > > > > It contains a bugfix for the weird green artifacts in the > > > FileChooser. > > > > By the way, when I was refactoring the form parsing code recently > > I saw those same artifacts on my form test page. > > Please let me know, if that problem still exists on your test page. Unfortunately the bug had disappeared again just before I upgraded FLTK so I can't tell if it made any difference. I'll keep an eye out for it. When I've finished the form code split and refactoring I plan to valgrind dillo2 again. Regards, Jeremy Henty

June 2, 2008

fltk2 snapshot update

by Johannes.Hofmann＠gmx.de

On Mon, Jun 02, 2008 at 05:24:57PM +0100, Jeremy Henty wrote: > On Mon, Jun 02, 2008 at 04:26:55PM +0200, Johannes Hofmann wrote: > > > ... there is a new fltk2 snaphot (r6132) available. > > Thanks, I just upgraded. A nice change: the build now completes > successfully without X windows running (it used to build the library > but fail to build some of the apps, which was annoying). > > > It contains a bugfix for the weird green artifacts in the > > FileChooser. > > By the way, when I was refactoring the form parsing code recently I > saw those same artifacts on my form test page. They came and went as > I changed the code: sometimes they didn't appear at all, sometimes a > reload would get rid of them, sometimes I had to go back and forward > in the navigation history to get rid of them. Since I was not > touching any widget code I assumed I was triggering some sporadic bug > in Dillo or FLTK. Let's see if the new FLTK behaves any better! > This may well have been the same bug. UpBox was accessing uninitialized memory. Please let me know, if that problem still exists on your test page. Regards, Johannes

June 2, 2008

fltk2 snapshot update

by corvid＠lavabit.com

Jeremy wrote: > On Mon, Jun 02, 2008 at 04:26:55PM +0200, Johannes Hofmann wrote: > > BTW, shouldn't we point users directly to fltk.org for downloading > > fltk2? Otherwise people might think that dillo2 needs a modified > > version of fltk2. > > +1 . We shouldn't let anything discourage people from trying dillo2, > so anything that suggests they might need to patch other software is a > bad idea. Along these lines, I've been wondering whether it might be a good idea to list the required and optional libraries on the CVS page.

June 2, 2008

fltk2 snapshot update

by onepoint＠starurchin.org

On Mon, Jun 02, 2008 at 04:26:55PM +0200, Johannes Hofmann wrote: > ... there is a new fltk2 snaphot (r6132) available. Thanks, I just upgraded. A nice change: the build now completes successfully without X windows running (it used to build the library but fail to build some of the apps, which was annoying). > It contains a bugfix for the weird green artifacts in the > FileChooser. By the way, when I was refactoring the form parsing code recently I saw those same artifacts on my form test page. They came and went as I changed the code: sometimes they didn't appear at all, sometimes a reload would get rid of them, sometimes I had to go back and forward in the navigation history to get rid of them. Since I was not touching any widget code I assumed I was triggering some sporadic bug in Dillo or FLTK. Let's see if the new FLTK behaves any better! > I think we should update the download instructions. +1 > BTW, shouldn't we point users directly to fltk.org for downloading > fltk2? Otherwise people might think that dillo2 needs a modified > version of fltk2. +1 . We shouldn't let anything discourage people from trying dillo2, so anything that suggests they might need to patch other software is a bad idea. Regards, Jeremy Henty

June 2, 2008

fltk2 snapshot update

by Johannes.Hofmann＠gmx.de

Hi, at fltk.org there is a new fltk2 snaphot (r6132) available. It contains a bugfix for the weird green artifacts in the FileChooser. I think we should update the download instructions. BTW, shouldn't we point users directly to fltk.org for downloading fltk2? Otherwise people might think that dillo2 needs a modified version of fltk2. Cheers, Johannes

June 2, 2008

why is SELECT a block container in Tags[]?

by corvid＠lavabit.com

Jorge wrote: > > > On Sat, May 31, 2008 at 07:02:02PM +0000, corvid wrote: > > > > <form> > > > > <select> > > > > <font color=red>segfault</font> > > > > </select> > > > > </form> > Patch committed (funny rendering sometimes, no segfault with > the above example). Justus got that sort of segfault because he must have standard_widget_colors set.

June 2, 2008

why is SELECT a block container in Tags[]?

by jcid＠dillo.org

On Sun, Jun 01, 2008 at 08:16:49PM +0000, corvid wrote: > Jorge wrote: > > On Sat, May 31, 2008 at 07:02:02PM +0000, corvid wrote: > > > Jorge wrote: > > > > On Sat, May 31, 2008 at 03:26:50PM +0000, corvid wrote: > > > > > Why is SELECT a block container rather than an inline container in Tags[]? > > > > > > > > Most probably by mistake! > > > > > > > > Given that the DTD says: > > > > > > > > <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> > > > > [...] > > > > <!ELEMENT OPTGROUP - - (OPTION)+ -- option group --> > > > > [...] > > > > <!ELEMENT OPTION - O (#PCDATA) -- selectable choice --> > > > > > > > > it looks like inline container would fit better. > > > > (and stop some segfaults Justus detected). > > > > > > Well, it would still provide him with new segfaults for inline stuff like > > > > > > <form> > > > <select> > > > <font color=red>segfault</font> > > > </select> > > > </form> > > > > BTW, patch committed (now SELECT is an inline container). > > Now dillo can't deal with multiple <option>s without </option> in between, > e.g., http://www.dillo.org/bugtrack/Dquery.html Oh, yes. Patch committed (funny rendering sometimes, no segfault with the above example). It looks simpler to have SELECT as a block container, but I prefer this simple patch because it _is_ an inline container. The decision on what to do with unallowed elements is yet to be made (ignore & warn or close the open container for instance). -- Cheers Jorge.-

June 2, 2008

why is SELECT a block container in Tags[]?

by corvid＠lavabit.com

Jorge wrote: > On Sat, May 31, 2008 at 07:02:02PM +0000, corvid wrote: > > Jorge wrote: > > > On Sat, May 31, 2008 at 03:26:50PM +0000, corvid wrote: > > > > Why is SELECT a block container rather than an inline container in Tags[]? > > > > > > Most probably by mistake! > > > > > > Given that the DTD says: > > > > > > <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> > > > [...] > > > <!ELEMENT OPTGROUP - - (OPTION)+ -- option group --> > > > [...] > > > <!ELEMENT OPTION - O (#PCDATA) -- selectable choice --> > > > > > > it looks like inline container would fit better. > > > (and stop some segfaults Justus detected). > > > > Well, it would still provide him with new segfaults for inline stuff like > > > > <form> > > <select> > > <font color=red>segfault</font> > > </select> > > </form> > > BTW, patch committed (now SELECT is an inline container). Now dillo can't deal with multiple <option>s without </option> in between, e.g., http://www.dillo.org/bugtrack/Dquery.html

June 1, 2008

Splitting html.cc: WAS: Re: [Dillo-dev] [patch]: replace form::Form ...

by onepoint＠starurchin.org

On Sun, Jun 01, 2008 at 01:42:11PM -0400, Jorge Arellano Cid wrote: > I removed form.{hh,cc} from cvs. Although they may reappear once > html.cc is split. Well, since you mention it...! :-) I had another go at splitting html.cc and this time I got it to work. No code was modified except that DilloHtml::getCurrentForm is no longer inline and the DilloHtmlForm constructor and destructor are wrapped in Html_form_{create,delete}, which can be called without access to the DilloHtmlForm class definition. So let's talk filenames! I moved the form code into new files called html_form.{hh,cc} . I really like hierarchical file names like that (they make related things sort next to each other), but maybe you would prefer the old form.{hh,cc} ? Also, I had to move a lot of declarations that both html.cc and html_form.cc use into a new header called html_internal.hh . Again, please specify a different file name if you want. I hadn't planned on creating a second new header but there are several definitions that are used by most of the html.cc internals, eg. almost everything needs the DilloHtml class definition and various HT2* macros etc. The "internal header" factors out that common API. I'd like to avoid it but it seems inevitable to me. Note that adding html_internal.hh is a one-off change. Further splitting should not require extra internal headers, just one new header per new source file as is standard. Also note that splitting off the form code is particularly messy since it includes new objects that appear in the DilloHtml definition. I would expect further splitting (eg, the table code) to be much simpler. The final decision is what to do with function names that move from *.cc to *.hh files. Leave them the same or rename "Html_*" to "a_Html_*"? I would suggest leaving them the same because they are still Html internals. They only appear in *.hh files because the internal implementation is spread among several files. If people think this sounds good and confirm the right names for files and functions then I will start proposing patches. I will move the code around in stages without breaking the build, so you'll get several small patches instead of one big one. I think that will be easier to review (but you can have a mega-patch if you really want one). Here are the stats: before: $ wc html*.{hh,cc} 21 42 338 html.hh 0 0 0 html_form.hh 0 0 0 html_internal.hh 6016 20229 191216 html.cc 0 0 0 html_form.cc 6037 20271 191554 total after: $ wc html*.{hh,cc} 21 42 338 html.hh 40 121 1404 html_form.hh 240 841 7832 html_internal.hh 4114 14090 129091 html.cc 1764 5563 57036 html_form.cc 6179 20657 195701 total (The final version may differ as I clean things up, but it won't be much different.) Regards, Jeremy Henty

June 1, 2008

Hooked a decoder for text/plain with charset.

by corvid＠lavabit.com

Jorge wrote: > On Fri, May 30, 2008 at 10:51:33PM +0000, corvid wrote: > > Jorge wrote: > > > On Thu, May 29, 2008 at 09:15:24PM +0000, corvid wrote: > > > > Jorge wrote: > > > > > On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > > > > > > Jorge wrote: > > > > > > > Just committed this patch. It makes things like: > > > > > > > > > > > > > > http://separat-lib.narod.ru/master.txt > > > > > > > > > > > > > > work. > > > > > > > > > > > > Why now? I don't understand the motivation for this. > > > > > > > > > > > > My next patch is just going to rip it out again because it > > > > > > will become unnecessary. > > > > > > > > > > Yeah, I noticed it could happen after making the patch, > > > > > but a segfault on that page made me fix and commit it. > > > > > > > > Why did it make plain segfault? > > > > > > The patch is attached for you to review in detail. > > > > > > The relevant part is an: > > > > > > Start_Ofs += i - len; > > > > > > substituted by a: > > > > > > Buf_Consumed = BufSize - len; > > > > > > (if the length of a long line got bigger than 'i', 'i - len' > > > was negative. A strange case triggered by that page, that has > > > really long lines). > > > > How did len get bigger than i? > > I tried the page yesterday without being able to get a crash. > > And now I've saved it locally and changed file.dpi to send it > > 200 bytes at a time, which gives me a long series of normal values > > when lines get bigger than packets: > > > > .. > > i-len 76 > > i-len 0 > > i-len 273 > > i-len 112 > > i-len 183 > > i-len 91 > > i-len 0 > > i-len 0 > > i-len 389 > > i-len 101 > > i-len 0 > > i-len 182 > > i-len 0 > > i-len 0 > > .. > > Well, it took me some time to reproduce it again... > > The attached patch is what I've got when adding the decoder over > the old code. It works most of the time, but if you go to: > > http://separat-lib.narod.ru/master.txt > > It will segfault with a Start_Ofs > BufSize. > Note: the patch is against current CVS. Oh, I thought the original no-decoder code was supposed to break.

June 1, 2008

Hooked a decoder for text/plain with charset.

by jcid＠dillo.org

On Fri, May 30, 2008 at 10:51:33PM +0000, corvid wrote: > Jorge wrote: > > On Thu, May 29, 2008 at 09:15:24PM +0000, corvid wrote: > > > Jorge wrote: > > > > On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > > > > > Jorge wrote: > > > > > > Just committed this patch. It makes things like: > > > > > > > > > > > > http://separat-lib.narod.ru/master.txt > > > > > > > > > > > > work. > > > > > > > > > > Why now? I don't understand the motivation for this. > > > > > > > > > > My next patch is just going to rip it out again because it > > > > > will become unnecessary. > > > > > > > > Yeah, I noticed it could happen after making the patch, > > > > but a segfault on that page made me fix and commit it. > > > > > > Why did it make plain segfault? > > > > The patch is attached for you to review in detail. > > > > The relevant part is an: > > > > Start_Ofs += i - len; > > > > substituted by a: > > > > Buf_Consumed = BufSize - len; > > > > (if the length of a long line got bigger than 'i', 'i - len' > > was negative. A strange case triggered by that page, that has > > really long lines). > > How did len get bigger than i? > I tried the page yesterday without being able to get a crash. > And now I've saved it locally and changed file.dpi to send it > 200 bytes at a time, which gives me a long series of normal values > when lines get bigger than packets: > > .. > i-len 76 > i-len 0 > i-len 273 > i-len 112 > i-len 183 > i-len 91 > i-len 0 > i-len 0 > i-len 389 > i-len 101 > i-len 0 > i-len 182 > i-len 0 > i-len 0 > .. Well, it took me some time to reproduce it again... The attached patch is what I've got when adding the decoder over the old code. It works most of the time, but if you go to: http://separat-lib.narod.ru/master.txt It will segfault with a Start_Ofs > BufSize. Note: the patch is against current CVS. -- Cheers Jorge.-

June 1, 2008

why is SELECT a block container in Tags[]?

by jcid＠dillo.org

On Sat, May 31, 2008 at 07:02:02PM +0000, corvid wrote: > Jorge wrote: > > On Sat, May 31, 2008 at 03:26:50PM +0000, corvid wrote: > > > Why is SELECT a block container rather than an inline container in Tags[]? > > > > Most probably by mistake! > > > > Given that the DTD says: > > > > <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> > > [...] > > <!ELEMENT OPTGROUP - - (OPTION)+ -- option group --> > > [...] > > <!ELEMENT OPTION - O (#PCDATA) -- selectable choice --> > > > > it looks like inline container would fit better. > > (and stop some segfaults Justus detected). > > Well, it would still provide him with new segfaults for inline stuff like > > <form> > <select> > <font color=red>segfault</font> > </select> > </form> BTW, patch committed (now SELECT is an inline container). With regard to further segfaults, quoting myself: > SELECT can be closed in Html_stack_cleanup_at_open(). SELECT > can contain OPTGROUP and OPTION. > > It would be good to consider the other cases before making a patch. After Justus sends us a set of segfault cases (with the fuzzy technique) we can try to tackle a general solution. -- Cheers Jorge.-

June 1, 2008

[patch]: replace form::Form and remove form.{cc,hh}

by jcid＠dillo.org

On Sat, May 31, 2008 at 05:33:50PM +0100, Jeremy Henty wrote: > > The source files dillo2/src/form.{hh,cc} (which define the form::Form > class) are modified copies of dw2/test/form.{hh,cc} . Dillo uses > almost none of the code in these files. All dillo needs is a Receiver > class that wraps a DilloHtmlForm and delegates calls to the event > handlers to DilloHtmlForm methods. > > This patch removes all references to form::Form and form.{hh,cc} . It > replaces form::Form with DilloHtmlReceiver. It also moves the event > handler code into a DilloHtmlForm method and removes some unnecessary > (void *) casting. This simplifies the code that bridges form events > to DilloHtmlForm methods. > > WARNING: developers must run " make maintainer-clean ; ./autogen.sh " > after this patch appears in their CVS working directory. This is > because automake has to recreate Makefile.in , otherwise the build > will try to compile form.cc which no longer works. > > After this patch is applied you can delete dillo2/src/form.{hh,cc} . > Don't delete dw2/test/form.{hh,cc} though! Committed. I removed form.{hh,cc} from cvs. Although the may reappear once html.cc is split. -- Cheers Jorge.-

June 1, 2008

why is SELECT a block container in Tags[]?

by corvid＠lavabit.com

Jorge wrote: > On Sat, May 31, 2008 at 03:26:50PM +0000, corvid wrote: > > Why is SELECT a block container rather than an inline container in Tags[]? > > Most probably by mistake! > > Given that the DTD says: > > <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> > [...] > <!ELEMENT OPTGROUP - - (OPTION)+ -- option group --> > [...] > <!ELEMENT OPTION - O (#PCDATA) -- selectable choice --> > > it looks like inline container would fit better. > (and stop some segfaults Justus detected). Well, it would still provide him with new segfaults for inline stuff like <form> <select> <font color=red>segfault</font> </select> </form>

May 31, 2008

why is SELECT a block container in Tags[]?

by jcid＠dillo.org

On Sat, May 31, 2008 at 03:26:50PM +0000, corvid wrote: > Why is SELECT a block container rather than an inline container in Tags[]? Most probably by mistake! Given that the DTD says: <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> [...] <!ELEMENT OPTGROUP - - (OPTION)+ -- option group --> [...] <!ELEMENT OPTION - O (#PCDATA) -- selectable choice --> it looks like inline container would fit better. (and stop some segfaults Justus detected). -- Cheers Jorge.-

May 31, 2008

[patch]: replace form::Form and remove form.{cc,hh}

by onepoint＠starurchin.org

The source files dillo2/src/form.{hh,cc} (which define the form::Form class) are modified copies of dw2/test/form.{hh,cc} . Dillo uses almost none of the code in these files. All dillo needs is a Receiver class that wraps a DilloHtmlForm and delegates calls to the event handlers to DilloHtmlForm methods. This patch removes all references to form::Form and form.{hh,cc} . It replaces form::Form with DilloHtmlReceiver. It also moves the event handler code into a DilloHtmlForm method and removes some unnecessary (void *) casting. This simplifies the code that bridges form events to DilloHtmlForm methods. WARNING: developers must run " make maintainer-clean ; ./autogen.sh " after this patch appears in their CVS working directory. This is because automake has to recreate Makefile.in , otherwise the build will try to compile form.cc which no longer works. After this patch is applied you can delete dillo2/src/form.{hh,cc} . Don't delete dw2/test/form.{hh,cc} though! Please comment! Regards, Jeremy Henty

May 31, 2008

why is SELECT a block container in Tags[]?

by corvid＠lavabit.com

Why is SELECT a block container rather than an inline container in Tags[]?

May 31, 2008

Hooked a decoder for text/plain with charset.

by corvid＠lavabit.com

Jorge wrote: > On Thu, May 29, 2008 at 09:15:24PM +0000, corvid wrote: > > Jorge wrote: > > > On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > > > > Jorge wrote: > > > > > Just committed this patch. It makes things like: > > > > > > > > > > http://separat-lib.narod.ru/master.txt > > > > > > > > > > work. > > > > > > > > Why now? I don't understand the motivation for this. > > > > > > > > My next patch is just going to rip it out again because it > > > > will become unnecessary. > > > > > > Yeah, I noticed it could happen after making the patch, > > > but a segfault on that page made me fix and commit it. > > > > Why did it make plain segfault? > > The patch is attached for you to review in detail. > > The relevant part is an: > > Start_Ofs += i - len; > > substituted by a: > > Buf_Consumed = BufSize - len; > > (if the length of a long line got bigger than 'i', 'i - len' > was negative. A strange case triggered by that page, that has > really long lines). How did len get bigger than i? I tried the page yesterday without being able to get a crash. And now I've saved it locally and changed file.dpi to send it 200 bytes at a time, which gives me a long series of normal values when lines get bigger than packets: .. i-len 76 i-len 0 i-len 273 i-len 112 i-len 183 i-len 91 i-len 0 i-len 0 i-len 389 i-len 101 i-len 0 i-len 182 i-len 0 i-len 0 ..

May 31, 2008

[patch]: html.cc: DilloHtmlInput::connectTo()

by jcid＠dillo.org

On Fri, May 30, 2008 at 05:33:17AM +0100, Jeremy Henty wrote: > > This implements DilloHtmlInput::connectTo() which is called by > DilloHtmlForm::addInput and connects the input to the form. > > This is another API milestone: now only DilloHtmlForm methods access > DilloHtmlForm::form_receiver . We are nearly done with the > DilloHtmlForm API, there only a few locations left that directly > access DilloHtmlForm data members. Committed. -- Cheers Jorge.-

May 30, 2008

[patch]: html.cc: remove DilloHtmlInput::widget

by jcid＠dillo.org

On Fri, May 30, 2008 at 03:22:50AM +0100, Jeremy Henty wrote: > > It turns out that the embed and widget members of the DilloHtmlInput > class are always the same, so we can remove widget. Patch attached. Committed. -- Cheers Jorge.-

May 30, 2008

[patch]: html.cc: DilloHtmlForm::getRadioInput()

by jcid＠dillo.org

On Thu, May 29, 2008 at 11:15:51PM +0100, Jeremy Henty wrote: > > This refactors code that looks for a radio input with a given name > into a new method DilloHtmlForm::getRadioInput() . > > This marks a DilloHtmlForm API milestone: now only DilloHtmlForm > member functions access the inputs data member. Everything else that > needs to get an input from a form goes through the API. Committed. -- Cheers Jorge.-

May 30, 2008

[patch]: html.cc: remove MSG_HTML and rename Html_msg() to DilloHtml::message()

by jcid＠dillo.org

On Fri, May 30, 2008 at 07:30:49PM +0100, Jeremy Henty wrote: > > Self-explanatory, I hope. This is a useful bit of yak-shaving to > prepare for more refactoring of form handling code. The reasons for > removing MSG_HTML are: > > * It doesn't belong in msg.h since it can only work when used in > http.cc . OK. > * It's a UI function not a debugging function like the other MSG_* > so it's misleading for it to look similar to the MSG_* macros. OK. > > * Using it has no advantage over simply calling the function that it > wraps. Yes it has. The macro is shorter and mnemonic. > * It's fragile because it requires html to be in scope and have the > right value. Easily detected at compile time. A mixed patch was committed. Left the method in DilloHtml and made a macro in html.cc. You should be able to continue from here. -- Cheers Jorge.-

May 30, 2008

Hooked a decoder for text/plain with charset.

by jcid＠dillo.org

Here goes the attachment... -- Cheers Jorge.-

May 30, 2008

Hooked a decoder for text/plain with charset.

by jcid＠dillo.org

On Thu, May 29, 2008 at 09:15:24PM +0000, corvid wrote: > Jorge wrote: > > On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > > > Jorge wrote: > > > > Just committed this patch. It makes things like: > > > > > > > > http://separat-lib.narod.ru/master.txt > > > > > > > > work. > > > > > > Why now? I don't understand the motivation for this. > > > > > > My next patch is just going to rip it out again because it > > > will become unnecessary. > > > > Yeah, I noticed it could happen after making the patch, > > but a segfault on that page made me fix and commit it. > > Why did it make plain segfault? The patch is attached for you to review in detail. The relevant part is an: Start_Ofs += i - len; substituted by a: Buf_Consumed = BufSize - len; (if the length of a long line got bigger than 'i', 'i - len' was negative. A strange case triggered by that page, that has really long lines). -- Cheers Jorge.-

May 30, 2008

[patch]: html.cc: remove MSG_HTML and rename Html_msg() to DilloHtml::message()

by onepoint＠starurchin.org

Self-explanatory, I hope. This is a useful bit of yak-shaving to prepare for more refactoring of form handling code. The reasons for removing MSG_HTML are: * It doesn't belong in msg.h since it can only work when used in http.cc . * It's a UI function not a debugging function like the other MSG_* so it's misleading for it to look similar to the MSG_* macros. * Using it has no advantage over simply calling the function that it wraps. * It's fragile because it requires html to be in scope and have the right value. Regards, Jeremy Henty

May 30, 2008

[patch]: html.cc: DilloHtmlInput::connectTo()

by onepoint＠starurchin.org

Oops, here's the patch. (I need a rest!) Jeremy Henty

May 30, 2008

[patch]: html.cc: DilloHtmlInput::connectTo()

by onepoint＠starurchin.org

This implements DilloHtmlInput::connectTo() which is called by DilloHtmlForm::addInput and connects the input to the form. This is another API milestone: now only DilloHtmlForm methods access DilloHtmlForm::form_receiver . We are nearly done with the DilloHtmlForm API, there only a few locations left that directly access DilloHtmlForm data members. Regards,

May 30, 2008

[patch]: html.cc: remove DilloHtmlInput::widget

by onepoint＠starurchin.org

It turns out that the embed and widget members of the DilloHtmlInput class are always the same, so we can remove widget. Patch attached. Regards, Jeremy Henty

May 30, 2008

[patch]: html.cc: DilloHtmlForm::getRadioInput()

by onepoint＠starurchin.org

This refactors code that looks for a radio input with a given name into a new method DilloHtmlForm::getRadioInput() . This marks a DilloHtmlForm API milestone: now only DilloHtmlForm member functions access the inputs data member. Everything else that needs to get an input from a form goes through the API. Regards, Jeremy Henty

May 30, 2008

Re: patch: cache Data allocation

by corvid＠lavabit.com

Jorge wrote: > On Thu, May 29, 2008 at 05:51:02PM +0000, corvid wrote: > > This is to make my unref()s cleaner. > > As it was in the prototype code, I would always unref at IOClose time. > > This made the reference count negative for file not found, which I didn't > > like. I could've tested for GotHeader, but it seemed more natural to > > test for data before unref()ing data. > > I'd prefer GotHeader because that's what Cache_process_queue() > tests for, and thus Data doesn't become an implicit flag. All right. > > I think it's all right for me to put that Cache_process_queue(entry) inside > > an entry->Data test -- at least I couldn't find anything bad happening. > > BTW, funny as it may seem, I don't yet know what data unref > you're talking about! :-) The reference counting in my charset-translating-in-cache code. (For any others in the audience: I keep track of how many things are reading the UTF8-ized version of the cache entry data.)

May 29, 2008

Hooked a decoder for text/plain with charset.

by corvid＠lavabit.com

Jorge wrote: > On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > > Jorge wrote: > > > Just committed this patch. It makes things like: > > > > > > http://separat-lib.narod.ru/master.txt > > > > > > work. > > > > Why now? I don't understand the motivation for this. > > > > My next patch is just going to rip it out again because it > > will become unnecessary. > > Yeah, I noticed it could happen after making the patch, > but a segfault on that page made me fix and commit it. Why did it make plain segfault?

May 29, 2008

Re: patch: cache Data allocation

by jcid＠dillo.org

On Thu, May 29, 2008 at 05:51:02PM +0000, corvid wrote: > This is to make my unref()s cleaner. > As it was in the prototype code, I would always unref at IOClose time. > This made the reference count negative for file not found, which I didn't > like. I could've tested for GotHeader, but it seemed more natural to > test for data before unref()ing data. I'd prefer GotHeader because that's what Cache_process_queue() tests for, and thus Data doesn't become an implicit flag. > I think it's all right for me to put that Cache_process_queue(entry) inside > an entry->Data test -- at least I couldn't find anything bad happening. BTW, funny as it may seem, I don't yet know what data unref you're talking about! :-) -- Cheers Jorge.-

May 29, 2008

Hooked a decoder for text/plain with charset.

by jcid＠dillo.org

On Thu, May 29, 2008 at 08:49:22PM +0000, corvid wrote: > Jorge wrote: > > Just committed this patch. It makes things like: > > > > http://separat-lib.narod.ru/master.txt > > > > work. > > Why now? I don't understand the motivation for this. > > My next patch is just going to rip it out again because it > will become unnecessary. Yeah, I noticed it could happen after making the patch, but a segfault on that page made me fix and commit it. -- Cheers Jorge.-

May 29, 2008

Hooked a decoder for text/plain with charset.

by corvid＠lavabit.com

Jorge wrote: > Just committed this patch. It makes things like: > > http://separat-lib.narod.ru/master.txt > > work. Why now? I don't understand the motivation for this. My next patch is just going to rip it out again because it will become unnecessary.

May 29, 2008

[patch]: simplify a_Html_form_event_handler(): part II

by jcid＠dillo.org

On Thu, May 29, 2008 at 07:38:26PM +0100, Jeremy Henty wrote: > > More simplification of form handling. Several functions that used an > index to an input in the form's input list have been changed to use a > pointer to the input, which removed some useless indirection. This > enabled refactoring the code in a_Html_form_event_handler() that found > the input from the resource into a new method "DilloHtmlInput > *DilloHtmlForm::getInput (void *v_resource)". Committed. -- Cheers Jorge.-

May 29, 2008

[patch]: simplify a_Html_form_event_handler()

by jcid＠dillo.org

On Thu, May 29, 2008 at 05:54:57PM +0100, Jeremy Henty wrote: > > The a_Html_form_event_handler() function displays an amusing > anti-pattern (which I wish I knew the name of) where it's not given > enough data and has to go searching for the missing information. > Specifically, it gets a pointer to the DilloHtml of the page, but not > not the DilloHtmlForm that submitted it, so it has to search the form > list of the DilloHtml to find the form. > > This patch passes the DilloHtmlForm to the handler and adds a pointer > from the DilloHtmlForm to the DilloHtml , so the handler can now > directly access the form and the html without messing around. Committed. -- Cheers Jorge.-

May 29, 2008

Hooked a decoder for text/plain with charset.

by jcid＠dillo.org

Hi, Just committed this patch. It makes things like: http://separat-lib.narod.ru/master.txt work. -- Cheers Jorge.-

May 29, 2008

[patch]: convert Html_submit_form2() to DilloHtmlForm::buildQueryUrl()

by jcid＠dillo.org

On Thu, May 29, 2008 at 03:40:02PM +0100, Jeremy Henty wrote: > > The function Html_submit_form2() does two jobs rolled into one: most > of its code creates a url from a DilloHtmlForm and a few lines push > the url to a DilloHtml . This patch takes advantage of the fact that > Html_submit_form2() is only called from a_Html_form_event_handler() ; > it moves the url push out into the caller and the rest of the code > into a new method "DilloUrl *DilloHtmlForm::buildQueryUrl()". This > separates concerns nicely: the DilloHtmlForm creates the url and > a_Html_form_event_handler() pushes it to the DilloHtml . Committed. > We're getting close to having a proper API for DilloHtmlForm , > unfortunately some of the remaining code is very messy. I'll do the > rest of the easy stuff and then summarise what's left. OK. -- Cheers Jorge.-

May 29, 2008

Previous patches a la camelCase

by jcid＠dillo.org

On Wed, May 28, 2008 at 05:29:35PM +0100, Jeremy Henty wrote: > > In case Jorge prefers them, here are revised versions of today's > patches with camelCased method names. Patch set Committed! -- Cheers Jorge.-

May 29, 2008

[patch]: html.cc: change Html_add_input() to HtmlForm::add_input()

by jcid＠dillo.org

On Wed, May 28, 2008 at 02:31:54PM +0100, Jeremy Henty wrote: > [...] > As before I'm submitting this as a "change diff" + "move > diff" to make it easier to review. Reviewers please tell me if it > makes things harder instead! So far it's been a pleasure to review in this format, but this is not a definitive answer because this is not a complex changeset. For the later case I'd like to have it munched like this anyway. ;) -- Cheers Jorge.-

May 29, 2008

[patch]: simplify a_Html_form_event_handler(): part II

by onepoint＠starurchin.org

More simplification of form handling. Several functions that used an index to an input in the form's input list have been changed to use a pointer to the input, which removed some useless indirection. This enabled refactoring the code in a_Html_form_event_handler() that found the input from the resource into a new method "DilloHtmlInput *DilloHtmlForm::getInput (void *v_resource)". Regards, Jeremy Henty

May 29, 2008

[patch]: simplify a_Html_form_event_handler()

by onepoint＠starurchin.org

The a_Html_form_event_handler() function displays an amusing anti-pattern (which I wish I knew the name of) where it's not given enough data and has to go searching for the missing information. Specifically, it gets a pointer to the DilloHtml of the page, but not not the DilloHtmlForm that submitted it, so it has to search the form list of the DilloHtml to find the form. This patch passes the DilloHtmlForm to the handler and adds a pointer from the DilloHtmlForm to the DilloHtml , so the handler can now directly access the form and the html without messing around. Regards, Jeremy Henty

May 29, 2008

[patch]: convert Html_submit_form2() to DilloHtmlForm::buildQueryUrl()

by onepoint＠starurchin.org

The function Html_submit_form2() does two jobs rolled into one: most of its code creates a url from a DilloHtmlForm and a few lines push the url to a DilloHtml . This patch takes advantage of the fact that Html_submit_form2() is only called from a_Html_form_event_handler() ; it moves the url push out into the caller and the rest of the code into a new method "DilloUrl *DilloHtmlForm::buildQueryUrl()". This separates concerns nicely: the DilloHtmlForm creates the url and a_Html_form_event_handler() pushes it to the DilloHtml . We're getting close to having a proper API for DilloHtmlForm , unfortunately some of the remaining code is very messy. I'll do the rest of the easy stuff and then summarise what's left. Regards, Jeremy Henty

May 29, 2008