Issues

ZF-5197: Decorator aliases are lost when Zend_Form_Element_Captcha adds its own decorators in 'render' method

Description

Reproduce code:

      $captcha = new Zend_Form_Element_Captcha('captcha', array(
            'captcha' => array(
                'captcha' => 'Image',
                'wordLen' => 6,
                'timeout' => 300,
                'font' => Zend_Registry::get('siteRootDir') . '/application/sketchh.ttf',
            )
        ));

        $captcha->setDecorators(array(
            //captcha decorator are placed at the begining of the array by default by 'render' method
            array('decorator' => array('foo' => 'HtmlTag'), 'options' => array('tag' => 'span')),                
            array('decorator' => array('bar' => 'HtmlTag'), 'options' => array('tag' => 'div')),
        ));

what we expect is to have something like this:

CAPTCHA

but the result is this:

CAPTCHA

The problem is that the aliases of the decorators (foo and bar) are lost while the 'render' method of Zend_Form_Element_Captcha adds its own decorators at the begining of the decorators array. This is done by "array_unshift" and then call to "setDecorators" but "setDecorators" in fact removes all current decorators and adds the new one with "addDecorators". There we have

foreach ($decorators as $decoratorInfo)

and here in fact the alias of the decorator is lost (in this case the alias is the index of the decorator in the array)

Comments

Looks like defect in Zend_Form_Element::addDecorators not in Captcha. Matthew, could you take a look?

Yes, as you correctly assumed, the problem is located in Zend_Form_Element, but in addDecorator() (without the s!)

The problem is that all decorators are given as objects, therefore addDecorator() will determine the classname and save the decorators with this classname as the index.

Therefore, if you use multiple decorators of the same kind, they will be overwritten.

Version: 1.8

You can add concrete instances using aliasing as well:


$form->addElement('text', 'foo', array(
    'decorators' => array(
        array(array('Foo' => new My_Decorator_Foo())),
    ),
));

You can even do it with addDecorator():

$element->addDecorator(array('Foo' => new My_Decorator_Foo())); ```

I have attached a patch which works to me. It changes the way Zend_Form_Element_Captcha::render() adds its decorators.

This issue still exists in 1.9.1

Matthew, I use to add my decorators in a derived formclass like this:

$this->setElementDecorators(array( array(array('biglabelClose' => 'HtmlTag'), array('tag' => 'div', 'closeOnly' => true, 'placement' => Zend_Form_Decorator_Abstract::PREPEND )), array('Label', array('placement' => Zend_Form_Decorator_Abstract::PREPEND)), array(array('biglabelOpen' => 'HtmlTag'), array('tag' => 'div', 'openOnly' => true, 'class' => 'biglabel', 'placement' => Zend_Form_Decorator_Abstract::PREPEND)) ) );

Doing it this way, the decorators with the aliases will be overwritten due to the way of saving in addDecorator.

However, the patch from Jesus works for me but should be reviewed; there have been a lot of whitespace changings.

@Christian H.: Of course it still exists in 1.9.1 - the patch has not yet been reviewed or applied.

@Matthew - why so many bug hunt days didn't resove that issue? It's quite old and annoying issue. Any plans to fix it in near future?

@Dominik -- Please be more polite when bumping issues. My own time during the bug hunt days is basically spent reviewing patch submissions and commits that occur during the bug hunt. The various participants in the bug hunt work on the issues that affect them personally typically. Additionally, if you look at the backlog of issues, the shear volume means that we simply will not get to them all on any given bug hunt.

One problem with this particular patch is that it does not include unit tests, which means that whomever considers applying it will also need to create a reproduce case and capture it as a test -- which means it's a non-trivial amount of time to apply.

If you want this issue resolved faster, help by submitting a unit test case.

The patch provided is incorrect. It may work in some cases but the changes made simply add the captcha decorators on top of the decorator stack. They should be rendered first and need to be at the bottom of the stack as is currently the case. Imo the proposed fix for case ZF-7552 is the way to go for this issue.

Change component

Fixed in r22464 and merged into 1.10 release branch