Zend Framework

Zend_Form_Decorators_FormErrors Doesnt Handle Manually Added Errors Properly

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.1
  • Fix Version/s: 1.10.5
  • Component/s: Zend_Form
  • Labels:
    None

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.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Rob Tuley added a comment -

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

Show
Rob Tuley added a comment - I just encountered the same problem. My workaround looked something like adding a extra elseif clause to handle these errors within _recurseForm...
Hide
Permalink
Reko Tiira added a comment -

Attached a patch that fixes the issue.

Show
Reko Tiira added a comment - Attached a patch that fixes the issue.
Hide
Permalink
Erwin Derksen added a comment -

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();
            }
        }
Show
Erwin Derksen added a comment - 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();
            }
        }
Hide
Permalink
Erwin Derksen added a comment -

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();
            }
        }
Show
Erwin Derksen added a comment - 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();
            }
        }
Hide
Permalink
Christian Albrecht added a comment -

Fixed in trunk r22272 and merged into 1.10 release branch

Show
Christian Albrecht added a comment - Fixed in trunk r22272 and merged into 1.10 release branch
Hide
Permalink
Christian Albrecht added a comment -

Fixed the fix in r22316 and added UnitTests

Show
Christian Albrecht added a comment - Fixed the fix in r22316 and added UnitTests

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.