Zend Framework

Zend_Form_Element_MultiCheckBox::addError() array problem

Details

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

Description

If you try to add an error to a Zend_Form_Element_MultiCheckBox element an php warning will occur:

Warning: Invalid argument supplied for foreach() in ........\library\Zend\Form\Element.php on line 2101

You can reproduce the error by executing this little snippet:

$element = new Zend_Form_Element_MultiCheckBox('someVar');
$element->addMultiOptions(array('hello', 'world'))
        ->setLabel("Hello world")
        ->setRequired(false)
        ->addError('someError');

Issue Links

This issue duplicates:
ZF-4915 multiCheckbox (_getErrorMessages()) fails if configuring custom validation message Major Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Tobias Petry added a comment -

There has been a bug in addError some time ago, which affected Zend_Element::addError (#ZF-3852).
Don't know if this is a help for anybody.

Show
Tobias Petry added a comment - There has been a bug in addError some time ago, which affected Zend_Element::addError (#ZF-3852). Don't know if this is a help for anybody.
Hide
Permalink
Tobias Petry added a comment -

The codeblock of the library is the following:

if ($this->isArray() || is_array($value)) {
    $aggregateMessages = array();
    foreach ($value as $val) {
        $aggregateMessages[] = str_replace('%value%', $val, $message);
    }
    $messages[$key] = $aggregateMessages;
}

The problem seems to be the if-clause. isArray is true, but $value is an empty string, when i run my snippet from above. So the source tries to iterate a string, which does not work and the php-warning for the for-each-loop is thrown.
I changed the if-clause to make an AND instead of an OR and it does work:

if ($this->isArray() && is_array($value)) {
    $aggregateMessages = array();
    foreach ($value as $val) {
        $aggregateMessages[] = str_replace('%value%', $val, $message);
    }
    $messages[$key] = $aggregateMessages;
}

So now, has $value to be an array if we want to loop.
It does work, but i don't know if i break something different, but i can't image it, because value HAS TO BE an array for iterating.

Show
Tobias Petry added a comment - The codeblock of the library is the following: 
if ($this->isArray() || is_array($value)) {
    $aggregateMessages = array();
    foreach ($value as $val) {
        $aggregateMessages[] = str_replace('%value%', $val, $message);
    }
    $messages[$key] = $aggregateMessages;
}
The problem seems to be the if-clause. isArray is true, but $value is an empty string, when i run my snippet from above. So the source tries to iterate a string, which does not work and the php-warning for the for-each-loop is thrown. I changed the if-clause to make an AND instead of an OR and it does work: 
if ($this->isArray() && is_array($value)) {
    $aggregateMessages = array();
    foreach ($value as $val) {
        $aggregateMessages[] = str_replace('%value%', $val, $message);
    }
    $messages[$key] = $aggregateMessages;
}
So now, has $value to be an array if we want to loop. It does work, but i don't know if i break something different, but i can't image it, because value HAS TO BE an array for iterating.
Hide
Permalink
Artur Bodera added a comment -

This also breaks when using addErrorMessages()
So there is no good way to display errors with this field.

See also: ZF-5603

Show
Artur Bodera added a comment - This also breaks when using addErrorMessages() So there is no good way to display errors with this field. See also: ZF-5603
Hide
Permalink
Artur Bodera added a comment -

When using addErrorMessages() the bug is in Zend_Form_Element::_getErrorMessages().

When we add error with Zend_Form_Element::addError() the following is executed:

/**
* Add an error message and mark element as failed validation
*
* @param  string $message
* @return Zend_Form_Element
*/
public function addError($message)
{
   $this->addErrorMessage($message);
   $this->markAsError();
   return $this;
}

Let's take a look at markAsError() function:

$messages       = $this->getMessages();
	$customMessages = $this->_getErrorMessages();
	$messages       = $messages + $customMessages;
	if (empty($messages)) {
		$this->_isError = true;
	} else {
		$this->_messages = $messages;
	}
	return $this;

The problem is in _getErrorMessages function. The resulting $message var, in case of Multi field will look like this:

$messages = array(
	0 => array(
		0 => 'My custom errormessage',
		1 => 'My custom errormessage',
		2 => 'My custom errormessage'
	)
)

It's structure is unknown for Zend_View_Helper_FormErrors, therefore there are errors.

I strongly suggest to give full control over the method of validating Multi elements! As for now validators are run on each and every selected
value, but there is no way to validate THE WHOLE FIELD, nor give it a GLOBAL SINGLE custom error message. Current architecture does NOT allow i.e. to
create a Zend_Validate_Count which would throw error if less (or more) number of fields have been selected.

Show
Artur Bodera added a comment - When using addErrorMessages() the bug is in Zend_Form_Element::_getErrorMessages(). When we add error with Zend_Form_Element::addError() the following is executed: 
/**
* Add an error message and mark element as failed validation
*
* @param  string $message
* @return Zend_Form_Element
*/
public function addError($message)
{
   $this->addErrorMessage($message);
   $this->markAsError();
   return $this;
}
Let's take a look at markAsError() function: 
$messages       = $this->getMessages();
	$customMessages = $this->_getErrorMessages();
	$messages       = $messages + $customMessages;
	if (empty($messages)) {
		$this->_isError = true;
	} else {
		$this->_messages = $messages;
	}
	return $this;
The problem is in _getErrorMessages function. The resulting $message var, in case of Multi field will look like this: 
$messages = array(
	0 => array(
		0 => 'My custom errormessage',
		1 => 'My custom errormessage',
		2 => 'My custom errormessage'
	)
)
It's structure is unknown for Zend_View_Helper_FormErrors, therefore there are errors. I strongly suggest to give full control over the method of validating Multi elements! As for now validators are run on each and every selected value, but there is no way to validate THE WHOLE FIELD, nor give it a GLOBAL SINGLE custom error message. Current architecture does NOT allow i.e. to create a Zend_Validate_Count which would throw error if less (or more) number of fields have been selected.
Hide
Permalink
Tobias Petry added a comment -

This patch resolves the problem.

Show
Tobias Petry added a comment - This patch resolves the problem.
Hide
Permalink
Tobias Petry added a comment -

added new patch with unit tests.

current Zend_Form trunk is defect, test with 1.9.2 final, it's working with no side effects.

Show
Tobias Petry added a comment - added new patch with unit tests. current Zend_Form trunk is defect, test with 1.9.2 final, it's working with no side effects.
Hide
Permalink
Tobias Petry added a comment -

fixed with #ZF-4915

Show
Tobias Petry added a comment - fixed with #ZF-4915

People

Vote (4)
Watch (2)

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.