Zend Framework

Decorator aliases are lost when Zend_Form_Element_Captcha adds its own decorators in 'render' method

Details

  • Type: Sub-task Sub-task
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.1
  • Fix Version/s: 1.10.6
  • Component/s: Zend_Form
  • Labels:
    None

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:
<div><span>CAPTCHA</span></div>

but the result is this:
<div>CAPTCHA</div>

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)

Issue Links

This issue is duplicated by:
ZF-7552 Same type decorators are lost when getting and setting back decorators Major Resolved
ZF-9338 Zend_Form_Element_Captcha loads two decorators in its render() function shd be in loadDefaultDecorator(); Major Resolved
This issue is related to:
ZF-9168 Zend Form Element Captcha Decorator Major Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Stanislav Malyshev added a comment -

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

Show
Stanislav Malyshev added a comment - Looks like defect in Zend_Form_Element::addDecorators not in Captcha. Matthew, could you take a look?
Hide
Permalink
Christian Heinrich added a comment -

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

Show
Christian Heinrich added a comment - 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
Hide
Permalink
Matthew Weier O'Phinney added a comment -

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()));



 

    

    

        Show
        

            
    
    
    
                

    Matthew Weier O'Phinney
 added a comment  -                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()));





         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Jesus Ruiz-Ayucar Vazquez
 added a comment  -   

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

    

    

        Show
        

            
    
    
    
                

    Jesus Ruiz-Ayucar Vazquez
 added a comment  -                I have attached a patch which works to me. It changes the way Zend_Form_Element_Captcha::render() adds its decorators.         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Christian Heinrich
 added a comment  -   

        
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.
 

    

    

        Show
        

            
    
    
    
                

    Christian Heinrich
 added a comment  -                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.         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Matthew Weier O'Phinney
 added a comment  -   

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

    

    

        Show
        

            
    
    
    
                

    Matthew Weier O'Phinney
 added a comment  -                @Christian H.: Of course it still exists in 1.9.1 - the patch has not yet been reviewed or applied.         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Dominik Bulaj
 added a comment  -   

        
@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?
 

    

    

        Show
        

            
    
    
    
                

    Dominik Bulaj
 added a comment  -                @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?         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Matthew Weier O'Phinney
 added a comment  -   

        
@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.
 

    

    

        Show
        

            
    
    
    
                

    Matthew Weier O'Phinney
 added a comment  -                @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.         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Hendri Smit
 added a comment  -   

        
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.
 

    

    

        Show
        

            
    
    
    
                

    Hendri Smit
 added a comment  -                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.         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Mickael Perraud
 added a comment  -   

        
Change component
 

    

    

        Show
        

            
    
    
    
                

    Mickael Perraud
 added a comment  -                Change component         

    



                     

    

        Hide
        

            Permalink
                                

        

    
    
    
                

    Christian Albrecht
 added a comment  -   

        
Fixed in r22464 and merged into 1.10 release branch
 

    

    

        Show
        

            
    
    
    
                

    Christian Albrecht
 added a comment  -                Fixed in r22464 and merged into 1.10 release branch         

    



                 



                

            

        


        

            

                
                    
People



                Vote
    
                (4)
    



            Watch
    
        
            (6)
    


Dates
    
    
  • 
            
    
            
    Created:
    
            
    
                        
                         
        
    
            
    
            
    Updated:
    
            
    
                        
                         
        
    
            
    
            
    Resolved:
    
            
    
                        
                         
        
    
        
    • 




                

            

        

        
            
                
            
        

    

















        Atlassian JIRA
        (v4.4.3#663-r165197)
        | Report a problem
    

            

                    Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.