ZF-10803: Form ID overrides Fieldset ID and causes duplicate IDs

Issue Type:
Bug
Created:
2010-12-09T05:50:32.000+0000
Last Updated:
2012-06-15T14:16:04.000+0000
Status:
Resolved
Fix version(s):
  • 1.12.0 (27/Aug/12)
Reporter:
Mehdi Rande (mehdi.rande)
Assignee:
Frank Brückner (frosch)
Tags:
  • Zend_Form
  • FixForZF1.12
  • state:patch-ready-for-review
  • zf-crteam-padraic
  • zf-crteam-priority
  • zf-crteam-review
Related issues:
Attachments:

Description

The form id still overrides fieldset id but only if the fieldset id is not set.

This code :


$form = new Zend_Form();
$form->setDecorators(
    array(
        'FormElements',
        array('Fieldset'),
        'Form',
    )
);
$form->setAttrib('id', 'form-id');
$this->view->form = $form;

Produces this form:

The fix in ZF-10679 just prevent the fieldset id to be overrides by the form's one if the fieldset id is set. But if the fieldset id is not set the form id still override it.

Comments

Posted by Mehdi Rande (mehdi.rande) on 2010-12-09T05:53:24.000+0000

Those two patch can fixe the bug, i do prefer the first one but both works


Index: Fieldset.php
===================================================================
--- Fieldset.php    (revision 23482)
+++ Fieldset.php    (working copy)
@@ -131,7 +131,7 @@
         $name    = $element->getFullyQualifiedName();
         $id      = (string)$element->getId();
 
-        if (!array_key_exists('id', $attribs) && '' !== $id) {
+        if ((!array_key_exists('id', $attribs) || $attribs['id'] == $id) && '' !== $id) {
             $attribs['id'] = 'fieldset-' . $id;
         }


Index: Fieldset.php
===================================================================
--- Fieldset.php    (revision 23482)
+++ Fieldset.php    (working copy)
@@ -72,6 +72,7 @@
         $options = parent::getOptions();
         if (null !== ($element = $this->getElement())) {
             $attribs = $element->getAttribs();
+            if (array_key_exists('id', $attribs)) unset($attribs['id']);
             $options = array_merge($attribs, $options);
             $this->setOptions($options);
         }

Posted by Ondrej Machulda (ondram) on 2011-06-09T11:40:04.000+0000

Huh, fix exists for year and a half, but still not in upstream :-(?

Posted by emilio (emiliodeg) on 2011-11-04T18:11:30.000+0000

come on guys! it's very easy to fix that

Posted by Frank Brückner (frosch) on 2011-11-05T09:18:09.000+0000

Fix and unit test added.

Posted by Martin Hujer (mhujer) on 2012-05-19T14:19:56.000+0000

"rebased" the patches, so it can be applied to latest trunk (r24811)

Posted by Frank Brückner (frosch) on 2012-05-31T10:13:22.000+0000

Thanks to Martin.

Posted by Adam Lundrigan (adamlundrigan) on 2012-06-15T14:16:04.000+0000

Fixed in trunk (1.12.0) r24961