ZF-10056: getErrors() add getMessages() does not filter out ignored form elements

Description

Zend_Form have ability to set certain form elements as "ignored" so no values will be taken from them. However when I call getErrors() or getMessages() methods - I get messages also from ignored form fields while, logically, I should not.

Comments

This issue stays unchanged for a year already and still available in latest 1.11.7 version. I've prepared patch for this issue to get it fixed at last. Since there seems to be now way to add attachments to issues - here is link to it: http://dl.dropbox.com/u/7763774/ZF-10056.patch

Hi Alexander,

Thanks for the patch to contribute to ZF, but I notice that you don't appear to have a CLA on file, if you do, you should get in touch with Ralph Schindler and ask him to assign you the correct groups so that you can attach patches as an attachment rather than inline, otherwise, you should sign the cla (http://framework.zend.com/wiki/display/…) and return it before contributing code, otherwise your contributions may go unused!

The patch does not respect the coding standards!

Zend Framework Coding Standard for PHP: Coding Style - Control Statements

Hello,

Kim, I've signed CLA and sent it for review, hope it will be accepted soon. Kai, yes, you right, here is fixed version of patch: http://dl.dropbox.com/u/7763774/…

Alexander,

Nice, I will test your patch tomorrow.

-Alexander, unfortunately I was not able to reproduce this issue. Please check my unit test if I'm missing something.-

Sorry there was an error in my test. Your patch seems to resolve the issue. However, I would truncate the patch to be


--- Zend/Form.php   Fri Jun 03 17:48:45 2011
+++ Zend/Form.php   Thu Jun 09 12:30:39 2011
@@ -2242,6 +2242,9 @@
         }
         $context = $data;
         foreach ($this->getElements() as $key => $element) {
+            if ($element->getIgnore()) {
+                continue;
+            }
             if (null !== $translator && $this->hasTranslator()
                     && !$element->hasTranslator()) {
                 $element->setTranslator($translator);

Yes, Kim, you right, however I would propose to change patch this way:


--- Zend/Form.php   Fri Jun 03 17:48:45 2011
+++ Zend/Form.php   Fri Jun 10 13:54:01 2011
@@ -2242,6 +2242,9 @@
         }
         $context = $data;
         foreach ($this->getElements() as $key => $element) {
+            if ($element->getIgnore()) {
+                continue;
+            }
             if (null !== $translator && $this->hasTranslator()
                     && !$element->hasTranslator()) {
                 $element->setTranslator($translator);
@@ -2300,6 +2303,9 @@
         $context    = $data;

         foreach ($this->getElements() as $key => $element) {
+            if ($element->getIgnore()) {
+                continue;
+            }
             $check = $data;
             if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
                 $check = $this->_dissolveArrayValue($data, $belongsTo);

because Zend_Form have 2 methods for validation - isValid() and isValidPartial().

Patch for fixing issue for isValidPartial() too

Alexander, I agree with the latest proposal. I have modified unit test to test also {{isValidPartial()}} and packed both the patch and the test in one file.

Are there any situations where it makes sense that a transient field would be ignored (ie: not be included in {{getValues()}}) but still need to be validated? My thought process goes along the lines of a CSRF element, which isn't part of the form's data (so would be excluded from {{getValues()}}) but still needs to be validated to ensure it's value is correct. If that is a viable use case, then we'll need to rethink the suggested fix.

I've linked this issue with ZF-6909 because in the comments (specifically here: http://framework.zend.com/issues/browse/… and here: http://framework.zend.com/issues/browse/…) there is a discussion about {{ignore}} vs {{disabled}} and how {{isValid}}/{{isValidPartial}} don't check the {{ignore}} flag. An opinion from Matthew was requested on the matter, but he did not respond.

bq. My thought process goes along the lines of a CSRF element, which isn't part of the form's data (so would be excluded from getValues()) but still needs to be validated to ensure it's value is correct. If that is a viable use case, then we'll need to rethink the suggested fix.

Adam - this is absolutely a valid argument and definitely the fix needs to be rethought. Fortunately someone has eyes open :)