ZF-5385: Zend_Form_Decorators_FormErrors Doesnt Handle Manually Added Errors Properly

Issue Type:
Bug
Created:
2008-12-30T16:44:59.000+0000
Last Updated:
2010-05-29T03:11:20.000+0000
Status:
Resolved
Fix version(s):
  • 1.10.5 (26/May/10)
Reporter:
Brad Gushurst (bradgman)
Assignee:
Christian Albrecht (alab)
Tags:
  • Zend_Form
Related issues:
Attachments:

Description

Zend_Form

The addErrorMessage method on line #2075 adds an error message to the $_errorMessage array without a key value, hence automatically getting a numeric key.

Zend_Form_Decorator_FormErrors

The _recurseForm method on line #366 parses through a list of errors including the $_errorMessage array from the form to put together the errors. The error messages added through the addErrorMessage method will not get picked up through here as its $key => $value pairs will not meet any of the conditions within the loop.

Solutions:

  1. Modify the recurse form loop to handle elements with the numeric key.

  2. Specify a key in the addErrorMessage method and specifically handle that key within the _recurseForm method as well.

Comments

Posted by Rob Tuley (robtuley) on 2009-06-03T03:12:14.000+0000

I just encountered the same problem. My workaround looked something like adding a extra elseif clause to handle these errors within _recurseForm...

Posted by Reko Tiira (reko_t) on 2009-12-28T03:10:33.000+0000

Attached a patch that fixes the issue.

Posted by Erwin Derksen (erwind) on 2010-03-31T05:33:14.000+0000

Imho, the patch has several drawbacks:

  • hasErrorMessages seems superfluous as the existing method isErrors seems more appropriate (in combination with using addError() instead of addErrorMessage())
  • the FormError decorator should render all messages, either being related or unrelated to an element. It should do so, regardless of how the form handles this (see next point). The patch renders either the custom messages or the elements/subforsm messages (as previously).
  • the Form itself also acts a bit strange in only returning either the custom messages or the element/subform related messages. This should be configurable.

So I changed the patch to


        foreach ($errors as $name => $list) {
            if (is_int($name)) {
                $content .= $this->getMarkupListStart()
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListEnd();
                }
            else if (null !== ($element = $form->getElement($name))) {
                $element->setView($view);
                $content .= $this->getMarkupListItemStart()
                         .  $this->renderLabel($element, $view)
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListItemEnd();
            } elseif (!$this->ignoreSubForms() && nul !== ($subForm = $form->getSubForm($name))) {
                $content .= $this->getMarkupListStart()
                         .  $this->_recurseForm($subForm, $view)
                         .  $this->getMarkupListEnd();
            }
        }

Posted by Erwin Derksen (erwind) on 2010-03-31T06:12:39.000+0000

I'm sorry, but the code I posted half an hour ago does not render valid html (

<

ul> directly within

<

ul>). Better is to use the code below.

note that the subform branch in the current version (1.10.2) contains the same error! (I'm not reporting that separately as this issue is open and major, so should receive enough attention). I changed it for the subform branch as well. However, I don't use subforms, so I'm not sure about that branch. If you want errors from a subform in their own list (to hide or expand them using javascript/css use the second code part.


        foreach ($errors as $name => $list) {
            if (is_int($name)) {
                $content .= $this->getMarkupListItemStart()
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListItemEnd();
                }
            else if (null !== ($element = $form->getElement($name))) {
                $element->setView($view);
                $content .= $this->getMarkupListItemStart()
                         .  $this->renderLabel($element, $view)
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListItemEnd();
            } else if (!$this->ignoreSubForms() && nul !== ($subForm = $form->getSubForm($name))) {
                $content .= $this->_recurseForm($subForm, $view);
            }
}


        foreach ($errors as $name => $list) {
           if (is_int($name)) {
                $content .= $this->getMarkupListItemStart()
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListItemEnd();
                }
            else if (null !== ($element = $form->getElement($name))) {
                $element->setView($view);
                $content .= $this->getMarkupListItemStart()
                         .  $this->renderLabel($element, $view)
                         .  $view->formErrors($list, $this->getOptions())
                         .  $this->getMarkupListItemEnd();
            } else if (!$this->ignoreSubForms() && nul !== ($subForm = $form->getSubForm($name))) {
                $content .= $this->getMarkupListItemStart()
                         .  $this->getMarkupListStart()
                         .  $this->_recurseForm($subForm, $view)
                         .  $this->getMarkupListEnd()
                         .  $this->getMarkupListItemEnd();
            }
        }

Posted by Christian Albrecht (alab) on 2010-05-24T08:56:42.000+0000

Fixed in trunk r22272 and merged into 1.10 release branch

Posted by Christian Albrecht (alab) on 2010-05-29T03:11:20.000+0000

Fixed the fix in r22316 and added UnitTests