Issues

ZF-11747: Zend_View_Helper_Form omits closing tag when no content is passed

Issue Type: Bug Created: 2011-09-18T00:16:41.000+0000 Last Updated: 2012-04-24T07:34:59.000+0000 Status: Resolved Fix version(s): - 1.11.11 (29/Sep/11)

Reporter: Rocky Koch (codercrush) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_View

  • zf-caretaker-adamlundrigan

Related issues: - ZF-11800

Attachments: - ZF-11747_fixedfiles.tar.gz

Description

The closing form tag is set only if the content is passed, so unlike false.

Comments

Posted by Adam Lundrigan (adamlundrigan) on 2011-09-23T03:03:01.000+0000

Reproducing Unit Test:

<pre class="highlight">
Index: Zend/View/Helper/FormTest.php
===================================================================
--- Zend/View/Helper/FormTest.php       (revision 24462)
+++ Zend/View/Helper/FormTest.php       (working copy)
@@ -167,6 +167,19 @@
         $form = $this->helper->form('FormName', array('action' => '/foo', 'method' => 'get'));
         $this->assertNotRegexp('/]*(name="FormName")/', $form);
     }
+
+    /**
+     * @group ZF-11747
+     */
+    public function testClosingTagIsPrintedWhenContentIsOmitted()
+    {
+        $this->view->doctype('XHTML11');
+        $form = $this->helper->form('FormName');
+        // Check that opening tag was printed
+        $this->assertContains('assertContains('', $form);
+    }
 }

 // Call Zend_View_Helper_FormTest::main() if this source file is executed directly.

Proposed fix:

<pre class="highlight">
Index: library/Zend/View/Helper/Form.php
===================================================================
--- library/Zend/View/Helper/Form.php   (revision 24462)
+++ library/Zend/View/Helper/Form.php   (working copy)
@@ -73,10 +73,11 @@
                . '>';

         if (false !== $content) {
-            $xhtml .= $content
-                   .  '';
+            $xhtml .= $content;
         }

+        $xhtml .= '';
+
         return $xhtml;
     }
 }

I don't see any harm in correcting this (obviously) incorrect behavior. Unless there are any objections, I will commit to trunk and merge to release-1.11.

Posted by Adam Lundrigan (adamlundrigan) on 2011-09-26T19:53:03.000+0000

Fixed in trunk r24477 Merged to release-1.11 in r24478

Posted by piet bijl (pbijl) on 2011-10-01T10:04:05.000+0000

I really dont understand this merge as this just broken numerous projects. Why was this implemented in the first place? For what gain? What was the problem? Why is it a "BUG" that you can render the form decorator WITH or WITHOUT a closing tag.

Now there is no way to render the form decorator APART from the other decorators. Now we are forced to buffer the output of custom form, and pass it as content to the form decorator.

Even when using a custom viewscript, there is no way to render the form now without the closing tag.

I ended up wrapping all the form decorators in a static function which removes the closing tag. Great decision.

No gain, alot of pain. Cool commit

Posted by piet bijl (pbijl) on 2011-10-01T10:12:24.000+0000

Please re-open, this craves for a revert.

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-01T23:32:38.000+0000

Apologies. That use case was missed while discussing and producing the fix, and the unit tests for Zend_View don't enforce this use case. I will add a test to enforce it so the problem doesn't reoccur.

Fix backed out from release (r24487) and trunk (r24488).

Posted by Michiel Thalen (chielsen) on 2011-10-04T08:31:47.000+0000

Just broke my project as well. Second bugfix release in 2 months that brakes my project :( Revert please.

Posted by Frank Br├╝ckner (frosch) on 2011-10-04T10:06:24.000+0000

@Michiel Please show us your code snippet which breaks your project. Thanks!

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-04T11:54:40.000+0000

I imagine it's this use case (from a view script), which I missed when vetting the fix:

<pre class="highlight">
<?php echo $this->form('foo'); ?>
    

In 1.11.11, this now produces the following HTML:

<pre class="highlight">

    

As the committed patch closes the tag regardless of whether or not content was passed into the form helper's third argument.

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-04T12:28:03.000+0000

I've attached two methods to fix the v1.11.11 release: * A tarball (ZF-11747_fixedfiles.tar.gz) containing full, fixed versions of both files. Apply:

<pre class="highlight">
 * A patch (ZF\-11747_revert.patch) which can be used to reverse the changes.  Apply:

Posted by Michiel Thalen (chielsen) on 2011-11-14T23:53:49.000+0000

This bug has been listed Resolved as not an issue? Well it is, and it seems to me that the fixed version is also in the trunk. So if that is true, maybe change this status. When is the release coming out?

Posted by Christiaan Kras (htbaa) on 2011-11-27T15:33:17.000+0000

I'd like to request this 'fix' to be reverted as well. In a big webapplication I use custom view scripts in which I call (from within the view) $this->form() with only 2 parameters, a form ID and an array with attributes, and later in the template I close the form with a manually inserted . I omit the 3rd parameter. Now this suddenly closes the form tag? I've been using this form since ZF 1.5. This release totally breaks my application, so I'll stick to a previous version for now.

Posted by Dominic Luechinger (dol) on 2011-11-30T14:49:11.000+0000

+1 for revert this modification. This fix/bug broke all some of our production code. We use a similar rendering to Adam Lundrigan comment. By passing false to the renderer, the closing form tag won't be added.

<pre class="highlight">
<?php echo $this->form->getDecorator('Form')->setElement($this->form)->render(false); ?>
    

Posted by Adam Lundrigan (adamlundrigan) on 2011-11-30T14:55:24.000+0000

This will be fixed in v1.11.12 and/or v1.12.0 (not sure if 1.11.12 is axed now that 1.12.x is coming down the pipe soon) Those who want to upgrade to v1.11.11 can, in the interim, follow my instructions above to revert the botched fix (here).

I do sincerely apologize for the inconvenience this has caused to everyone involved.

Posted by Dominic Luechinger (dol) on 2011-11-30T15:03:38.000+0000

Sorry, if wasn't reading carefully. It's fixed in r24487 and r24488. Thx for reverting. No problem at all. Was funny to debug. Was looking everywhere (different browser, javascript, ...). ;-)

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