Re: HTML_QuickForm Default Renderer...what the...?

On Fri, 11 Feb 2005 22:51:58 +0300, Alexey Borzov <borz_off@cs.msu.su> wrote: > Hi, > > Justin Patrin wrote: > > I just ran into a very strange bug pertaining to HTML_QuickForm's > > defaultRenderer method. I'm working on a system which embeds a form > > within an element and I lost most of my form (and it gets output very > > strangely). I finally tracked this down to the defaultRenderer method. > > Since this returns reference to a globally stored renderer and all of > > the methods which use this use references, the nested form picks up > > the same renderer as the parent form, causing the output to be reset, > > ending up with some very strange HTML. > > > > If I change this method to: > > > > function &defaultRenderer() > > { > > include_once('HTML/QuickForm/Renderer/Default.php'); > > $renderer =& new HTML_QuickForm_Renderer_Default(); > > return $renderer; > > } // end func defaultRenderer > > > > Everything works as expected. > > > > I can see from the HTML_QuickForm changelogs that this function was > > part of the original renderer system that was checked in (rev 1.72 of > > QuickForm.php I think) and was never changed since then. > > > > If I specify a renderer in my "sub-form" then everything works as > > expected as well. > > > > I propose this function be changed not to use a global so that things > > like what I'm trying to do will work. In addition, this will make QF > > work the same by default as if you specify your own renderer. > > I suggest you do the following first: > 1) Run examples (at least elements.php and groups.php) provided with QuickForm > package; > 2) Do the proposed change to the defaultRenderer() method; > 3) Run the examples again and observe some "subtle differences". > > The proposed change will cause a *major* BC break. > Ok, I see that now. However, this is IMHO still very broken. Granted the change can't be made now, but I would suggest instead of keeping an object as the default renderer, use a specific class. That way someone could extend the default renderer, make their changes, then set that *class* as the default renderer class. THen that class would be instantiated for every form which uses the default renderer. But too late now. I suppose I'll just have to work around it. -- Justin Patrin