Issues

ZF-9710: When having subforms FormErrors produce bad html markup.

Issue Type: Bug Created: 2010-04-19T09:35:34.000+0000 Last Updated: 2011-02-22T13:05:10.000+0000 Status: Resolved Fix version(s): - 1.11.4 (03/Mar/11)

Reporter: Sergio Medina (sergiomedinag) Assignee: Matthew Weier O'Phinney (matthew) Tags: - Zend_Form

Related issues: Attachments:

Description

This only happens when having subforms in a form with the Zend_Form_Decorator_FormErrors. To produce the error:

<pre class="highlight">
class Form_User extends Zend_Form
{
    public function init()
    {
        $this->addElement('text', 'name', array(
            'label' => 'Name',
            'required' => true,
            'validators' => array(
                new Zend_Validate_Alnum(true)
            )
        ));

        $this->addElement('text', 'email', array(
            'label' => 'Email',
            'required' => true,
            'validators' => array(
                new Zend_Validate_EmailAddress()
            )
        ));

        $subform = new Zend_Form_SubForm();

        $subform->addElement('text', 'address', array(
            'label' => 'Address',
            'required' => true,
            'validators' => array(
                new Zend_Validate_Alnum(true)
            )
        ));

        $subform->addElement('text', 'phone', array(
            'label' => 'Phone number',
            'required' => true
        ));

        $this->addSubForm($subform, 'location');

        $this->addElement('submit', 'submit');

        $this->addDecorators(array(
            new Zend_Form_Decorator_FormErrors(),
            new Zend_Form_Decorator_FormElements(),
            new Zend_Form_Decorator_HtmlTag(array(
                'tag' => 'dl', 'class' => 'zend_form'
            )),
            new Zend_Form_Decorator_Form(),
        ));
    }
}

When submiting with missing or wrong values it will produce:

<pre class="highlight">


Element Label

Errors
      
    Element Label

Errors
      

Subform element Label

Errors
        

The problem seems to be in the protected method _recurseSubform.

Comments

Posted by Christian Albrecht (alab) on 2010-05-24T09:12:14.000+0000

Fixed in trunk r22270 and merged into 1.10 release branch

Posted by Marian Böhmer (djmabo) on 2010-07-05T07:08:30.000+0000

It seems to be not fixed. Subform without elements produces:

Posted by B. Charbonneau (beeboo) on 2010-09-14T10:43:47.000+0000

There seems to be no check on if the subform is actually rendering any markup.

Mimicking the behavior from its own render() method, the following patch fixes the issue [~djmabo] is seeing

<pre class="highlight">
Index: FormErrors.php
===================================================================
--- FormErrors.php  (revision 22929)
+++ FormErrors.php  (working copy)
@@ -451,9 +451,13 @@
                              .  $this->getMarkupListItemEnd();
                 }
             } else if ($subitem instanceof Zend_Form && !$this->ignoreSubForms()) {
-                $content .= $this->getMarkupListStart()
-                          . $this->_recurseForm($subitem, $view)
-                          . $this->getMarkupListEnd();
+                $markup = $this->_recurseForm($subitem, $view);
+
+                if (!empty($markup)) {
+                    $content .= $this->getMarkupListStart()
+                              . $markup
+                              . $this->getMarkupListEnd();
+                }
             }
         }
         return $content;

Posted by B. Charbonneau (beeboo) on 2010-10-13T20:57:55.000+0000

Any comments on the proposed patch?

Posted by Fabio Almeida (fabius) on 2010-12-09T06:16:09.000+0000

I confirm this bug is still around in 1.11.1.

The proposed patch works fine for me.

Posted by B. Charbonneau (beeboo) on 2011-01-20T12:49:33.000+0000

I can confirm this bug is still around in 1.11.2.

This patch will adds a test case for this issue to the FormErrorsTest test class:

<pre class="highlight">Index: Zend/Form/Decorator/FormErrorsTest.php
===================================================================
--- Zend/Form/Decorator/FormErrorsTest.php  (revision 23642)
+++ Zend/Form/Decorator/FormErrorsTest.php  (working copy)
@@ -138,6 +138,18 @@
         $this->assertSame($content, $this->decorator->render($content));
     }
 
+    public function testNotGeneratingSubformErrorMarkupWrappingWhenNoErrors()
+    {
+        $form1 = new Zend_Form_SubForm();
+        $form2 = new Zend_Form();
+        $form2->addSubForm($form1, 'sub');
+        $form2->setView($this->getView());
+        $this->decorator->setElement($form2);
+
+        $content = 'test content';
+        $this->assertSame($content, $this->decorator->render($content));
+    }
+
     public function testRenderRendersAllErrorMessages()
     {
         $this->setupForm();
@@ -289,6 +301,7 @@
         $this->assertNotContains('form-badness', $html);
     }
 
+
     /**
      * @dataProvider markupOptionMethodsProvider
      */

Posted by Matthew Weier O'Phinney (matthew) on 2011-01-21T14:49:00.000+0000

Actually, that test case is flawed -- there's no way it can produce the value "test content" from rendering. The output actually rendered is "

" -- which makes sense to me. Can you clarify the test case?

Posted by B. Charbonneau (beeboo) on 2011-01-26T11:29:01.000+0000

I disagree. $this->decorator->render($content) does indeed render 'test content' (the FormErrors decorator prepends or appends error messages to the standard output of a form). A number of the other tests use a similar method calling render() (indeed, thats where I copied it from). The complete output is actually:

<pre class="highlight">
test content

 
    <pre class="highlight">
    test content


So, in summary, in the case where a form has no errors, the Zend\_Form\_Decorator\_FormErrors::render($content) merely returns $content.

 
    <pre class="highlight">
            $markup = $this->_recurseForm($form, $view);
    
            if (empty($markup)) {
                return $content;
            }


This is true for the case where a form has NO subforms.

In the case where a form has a subform, during the \_recurseForm method, the decorator ALWAYS wraps the output from the recursive response to \_recurseForm even when the response is empty.

 
    <pre class="highlight">
                } else if ($subitem instanceof Zend_Form && !$this->ignoreSubForms()) {
                    $content .= $this->getMarkupListStart()
                              . $this->_recurseForm($subitem, $view)
                              . $this->getMarkupListEnd();
                }


So when FormErrors decorates a form with no errors and no subforms, the return from render is merely the form itself. When FormErrors decorates a form with no errors and at least one subform also with NO errors, the return from render is "

- 

" (the inner

<

ul> from the lines 453-457 above, and the outer

<

ul> caused by code continuing past line 95 highlighted above).

This behavior is invalid, and I believe the test case I've provided correctly identifies the problem.

The patch I've proposed above checks makes \_recurseForm check for a non-empty response from $this->\_recurseForm($subitem, $view) before wrapping it with the error list start/end markup, just as render() does with the over-all response from \_recurseForm().

 

 

Posted by Matthew Weier O'Phinney (matthew) on 2011-02-22T13:05:08.000+0000

Unit test case added and patch applied.

 

 

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts