Component changed to Zend_Form. Priority changed because a simple workaround is available.

Please have a look at: ZF-8094 - Captcha element always set captcha decorator

Example to reproduce the problem.

$form = new Zend_Form(); 

$form->addElement( 
    'captcha', 
    'foo', 
    array( 
        'label' => 'Foo', 
            'captcha' => array( 
            'captcha' => 'Figlet', 
            'wordLen' => 6, 
            'timeout' => 300, 
        ), 
    ) 
); 

echo '

Before rendering

Output

After rendering

Before rendering

Output

After rendering

Before rendering

• Zend_Form_Decorator_Errors
• Zend_Form_Decorator_Description
• Zend_Form_Decorator_HtmlTag
• Zend_Form_Decorator_Label

After rendering

• Zend_Form_Decorator_Captcha_Word
• Zend_Form_Decorator_Errors
• Zend_Form_Decorator_Description
• Zend_Form_Decorator_HtmlTag
• Zend_Form_Decorator_Label

The decorator "Zend_Form_Decorator_Captcha" is missing.

BUT:

$form->getElement('foo')->clearDecorators(); 

Before rendering

Output

After rendering

Before rendering

(empty)

After rendering

• Zend_Form_Decorator_Captcha_Word

Thanks for opening this bug. It is affected by Subversion revision #24223 for library/Zend/Form/Element/Captcha.php which fixed ReCaptcha, but broke the other captchas (Figlet, Image and Dumb which are all Word Captchas). Specifically, the following block of code doesn't work well for Word Captchas (it only does for ReCaptcha):

$decorator  = $captcha->getDecorator();
if (!empty($decorator)) {
    array_unshift($decorators, $decorator);
} else {
     $decorator = array('Captcha', array('captcha' => $captcha));
     array_unshift($decorators, $decorator);
}

$this->setDecorators($decorators);

Recaptcha needs just his own decorator, so the if/else block is correct for it's case. All the other (word) captchas need both their own decorator (which is actually "Captcha_Word") AND additionally the generic Captcha Decorator, so that if/else block ruins it for them.

I can think of two different solutions, I don't know which one Zend Framework developers think as better:

• Change the above if/else block to a bit more complicated (and "uglier") one
• Refactor the Captcha_Word decorator (making it as "good" and "rich" as the Captcha_ReCaptcha decorator is) and (probably) drop the generic Captcha decorator completely. (I would say this is the cleaner approach but ZF devs should decide)

For the first case, the if/else could be something like the following pseudo-code:

$decorator  = $captcha->getDecorator();
if (!empty($decorator)) {
    array_unshift($decorators, $decorator);
}

if (the current captcha object is not ReCaptcha) {
     // push an additional decorator to the stack
     $decorator = array('Captcha', array('captcha' => $captcha));
     array_unshift($decorators, $decorator);
}

$this->setDecorators($decorators);

{quote} Refactor the Captcha_Word decorator (making it as "good" and "rich" as the Captcha_ReCaptcha decorator is) and (probably) drop the generic Captcha decorator completely. (I would say this is the cleaner approach but ZF devs should decide) {quote} I think it is a bad idea, because we need a separation between image/figlet and the text input.

(The example does not use the current implementation, because the current implementation adds additional decorators in the method to render!)

// Form
$form = new Zend_Form();
$form->removeDecorator('HtmlTag');

// Element
$form->addElement(
    'captcha',
    'foo',
    array(
        'label' => 'Foo',
        'captcha' => array(
            'captcha' => 'Figlet',
            'wordLen' => 6,
            'timeout' => 300,
        ),
    )
);

// Element decorators
$form->getElement('foo')->setDecorators(array(
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'spanOpen' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag'      => 'span',
            'openOnly' => true,
        ),
    ),
    // Zend_Form_Decorator_Captcha
    'Captcha',
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'spanClose' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag'       => 'span',
            'closeOnly' => true,
            'placement' => 'append',
        ),
    ),
    // Zend_Form_Decorator_Captcha_Word
    'Captcha_Word',
    // Zend_Form_Decorator_Errors
    'Errors',
    // Zend_Form_Decorator_Description
    'Description',
    // Zend_Form_Decorator_Label
    'Label',
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'div' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag' => 'div',
        ),
    ),
));

echo $form->render($view);

__    __   __   __   __   _    __   __    ______    _____   
\ \\ / //  \ \\/ // | || | ||  \ \\/ //  /_   _//  |  ___|| 
 \ \/ //    \ ` //  | '--' ||   \ ` //     | ||    | ||__   
  \  //      | ||   | .--. ||    | ||     _| ||    | ||__   
   \//       |_||   |_|| |_||    |_||    /__//     |_____|| 
    `        `-`'   `-`  `-`     `-`'    `--`      `-----`  
                                                            

@Thanos The new implementation must respect "loadDefaultDecoratorsIsDisabled()". See my example above with “clearDecorators()" and ZF-8094.

{quote} The new implementation must respect "loadDefaultDecoratorsIsDisabled()". See my example above with “clearDecorators()" and ZF-8094. {quote}

@Kai I agree, it's a chance to solve both problems/issues with one patch. I think that the Zend_Form_Element_Captcha render function currently doesn't show much respect neither for ReCaptcha nor for the others, am I correct ? It is the only render function (among other zend form elements) that actually pushes additional decorators in the stack.

Are the Zend_Form_Decorator_Captcha_Word, Zend_Form_Decorator_Captcha and Zend_Form_Decorator_Captcha_ReCaptcha regarded as default decorators or not ? Which of them should or shouldn't be in the default decorators group ?

{quote}I think that the Zend_Form_Element_Captcha render function currently doesn't show much respect neither for ReCaptcha nor for the others, am I correct ?{quote} Right. {quote}It is the only render function (among other zend form elements) that actually pushes additional decorators in the stack.{quote} Zend_Form_Element_File needs also a special decorator, but takes a different approach. {quote}Are the Zend_Form_Decorator_Captcha_Word, Zend_Form_Decorator_Captcha and Zend_Form_Decorator_Captcha_ReCaptcha regarded as default decorators or not ? Which of them should or shouldn't be in the default decorators group ?{quote} This is the problem: ||Captcha adapters||Required decorators|| |Dumb, Figlet, Image|Zend_Form_Decorator_Captcha + Zend_Form_Decorator_Captcha_Word| |ReCaptcha|Zend_Form_Decorator_Captcha_ReCaptcha|

All right, how about this patch:

--- library/Zend/Form/Element/Captcha.php.orig  2011-07-26 19:02:59.473494443 +0300
+++ library/Zend/Form/Element/Captcha.php   2011-07-26 19:21:08.795165709 +0300
@@ -178,17 +178,21 @@
         $captcha    = $this->getCaptcha();
         $captcha->setName($this->getFullyQualifiedName());
 
-        $decorators = $this->getDecorators();
+        if (!$this->loadDefaultDecoratorsIsDisabled()) {
+            $decorators = $this->getDecorators();
 
-        $decorator  = $captcha->getDecorator();
-        if (!empty($decorator)) {
-            array_unshift($decorators, $decorator);
-        } else {
-            $decorator = array('Captcha', array('captcha' => $captcha));
-            array_unshift($decorators, $decorator);
-        }
+            $decorator  = $captcha->getDecorator();
+            if (!empty($decorator)) {
+                array_unshift($decorators, $decorator);
+            }
+
+            if ($captcha instanceof Zend_Captcha_Word) {
+                $decorator = array('Captcha', array('captcha' => $captcha));
+                array_unshift($decorators, $decorator);
+            }
 
-        $this->setDecorators($decorators);
+            $this->setDecorators($decorators);
+        }
 
         $this->setValue($this->getCaptcha()->generate());
 

It makes the render function look like this:

public function render(Zend_View_Interface $view = null)
    {
        $captcha    = $this->getCaptcha();
        $captcha->setName($this->getFullyQualifiedName());

        if (!$this->loadDefaultDecoratorsIsDisabled()) {
            $decorators = $this->getDecorators();

            $decorator  = $captcha->getDecorator();
            if (!empty($decorator)) {
                array_unshift($decorators, $decorator);
            }

            if ($captcha instanceof Zend_Captcha_Word) {
                $decorator = array('Captcha', array('captcha' => $captcha));
                array_unshift($decorators, $decorator);
            }

            $this->setDecorators($decorators);
        }

        $this->setValue($this->getCaptcha()->generate());

        return parent::render($view);
    }

Should we check for existing captcha decorators?

/**
 * @group ZF-11609
 */
public function testDecoratorsBeforeAndAfterRendering()
{
    // Disable default decorators is true
    $element = new Zend_Form_Element_Captcha(
        'foo',
        array(
            'captcha'        => 'Dumb',
            'captchaOptions' => array(
                'sessionClass' => 'Zend_Form_Element_CaptchaTest_SessionContainer',
            ),
            'disableLoadDefaultDecorators' => true,
            'decorators'                   => array(
                'Description',
                'Errors',
                'Captcha_Word',
                'Captcha',
                'Label',
            ),
        )
    );

    // Before rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    $element->render();

    // After rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    // Disable default decorators is false
    $element = new Zend_Form_Element_Captcha(
        'foo',
        array(
            'captcha'        => 'Dumb',
            'captchaOptions' => array(
                'sessionClass' => 'Zend_Form_Element_CaptchaTest_SessionContainer',
            ),
            'decorators' => array(
                'Description',
                'Errors',
                'Captcha_Word',
                'Captcha',
                'Label',
            ),
        )
    );

    // Before rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    $element->render();
    
    // After rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );
}

There was 1 failure:

1) Zend_Form_Element_CaptchaTest::testDecoratorsBeforeAndAfterRendering
array (
  0 => 'Zend_Form_Decorator_Captcha',
  1 => 'Zend_Form_Decorator_Captcha_Word',
  2 => 'Zend_Form_Decorator_Description',
  3 => 'Zend_Form_Decorator_Errors',
  4 => 'Zend_Form_Decorator_Label',
)
--- Expected
+++ Actual
@@ @@
 Array
 (
-    [0] => Zend_Form_Decorator_Description
-    [1] => Zend_Form_Decorator_Errors
-    [2] => Zend_Form_Decorator_Captcha_Word
-    [3] => Zend_Form_Decorator_Captcha
+    [0] => Zend_Form_Decorator_Captcha
+    [1] => Zend_Form_Decorator_Captcha_Word
+    [2] => Zend_Form_Decorator_Description
+    [3] => Zend_Form_Decorator_Errors
     [4] => Zend_Form_Decorator_Label
 )

(I would upload the patch as an attachment, but perhaps I don't have the permissions to do so). Right, here is an improved version that should not fail the above Unit test (it checks for existing captcha decorators):

--- library/Zend/Form/Element/Captcha.php.orig  2011-07-26 19:02:59.473494443 +0300
+++ library/Zend/Form/Element/Captcha.php   2011-07-27 13:17:00.216725827 +0300
@@ -178,17 +178,24 @@
         $captcha    = $this->getCaptcha();
         $captcha->setName($this->getFullyQualifiedName());
 
-        $decorators = $this->getDecorators();
+        if (!$this->loadDefaultDecoratorsIsDisabled()) {
+            $decorators = $this->getDecorators();
+            $decorator  = $captcha->getDecorator();
+            $key        = get_class($this->_getDecorator($decorator, null));
+
+            if (!empty($decorator) && !array_key_exists($key, $decorators)) {
+                array_unshift($decorators, $decorator);
+            }
 
-        $decorator  = $captcha->getDecorator();
-        if (!empty($decorator)) {
-            array_unshift($decorators, $decorator);
-        } else {
             $decorator = array('Captcha', array('captcha' => $captcha));
-            array_unshift($decorators, $decorator);
-        }
+            $key       = get_class($this->_getDecorator($decorator[0], $decorator[1]));
 
-        $this->setDecorators($decorators);
+            if ($captcha instanceof Zend_Captcha_Word && !array_key_exists($key, $decorators)) {
+                array_unshift($decorators, $decorator);
+            }
+
+            $this->setDecorators($decorators);
+        }
 
         $this->setValue($this->getCaptcha()->generate());
 

It makes the render function look like this:

    public function render(Zend_View_Interface $view = null)
    {
        $captcha    = $this->getCaptcha();
        $captcha->setName($this->getFullyQualifiedName());

        if (!$this->loadDefaultDecoratorsIsDisabled()) {
            $decorators = $this->getDecorators();
            $decorator  = $captcha->getDecorator();
            $key        = get_class($this->_getDecorator($decorator, null));

            if (!empty($decorator) && !array_key_exists($key, $decorators)) {
                array_unshift($decorators, $decorator);
            }

            $decorator = array('Captcha', array('captcha' => $captcha));
            $key       = get_class($this->_getDecorator($decorator[0], $decorator[1]));

            if ($captcha instanceof Zend_Captcha_Word && !array_key_exists($key, $decorators)) {
                array_unshift($decorators, $decorator);
            }

            $this->setDecorators($decorators);
        }

        $this->setValue($this->getCaptcha()->generate());

        return parent::render($view);
    }

Fix and unit tests

@Thanos Kyritsis You will need to sign and submit a CLA before you can attach your patch to this issue See here: http://framework.zend.com/wiki/display/…

Hi Adam, Thanos wrote the patch. I've tested the patch, write the unit tests and added both files.

Patches reviewed and applied to both trunk and 1.11 release branch.