ZF-8265: Zend_Form_Element_Radio generates non valid html

Issue Type:
Bug
Created:
2009-11-09T17:17:10.000+0000
Last Updated:
2009-11-20T11:19:46.000+0000
Status:
Resolved
Fix version(s):
  • 1.9.6 (24/Nov/09)
Reporter:
Ernest Szulikowski (est)
Assignee:
Matthew Weier O'Phinney (matthew)
Tags:
  • Zend_Form
Related issues:
Attachments:

Description


        $this->addElement('radio', 'test', array(
            'label'    => 'test',
            'multiOptions' => array('Mr' => 'Mr','Ms' => 'Ms','Miss'
            => 'Miss','Mrs' => 'Mrs'),
        ));


//generated output
test

 Mr


Ms

Miss

Mrs

: 'disablefor' is not valid (X)HTML attribute in any case.

Proposed fix:

<

pre class="literal">

Index: FormLabel.php

--- FormLabel.php (revision 19018) +++ FormLabel.php (working copy) @@ -54,9 +54,13 @@ }

     $value = ($escape) ? $this->view->escape($value) : $value;

- $for = (empty($attribs['disableFor']) || !$attribs['disableFor']) - ? ' for="' . $this->view->escape($id) . '"' - : ''; + if (empty($attribs['disableFor']) || !$attribs['disableFor']) { + $for = ' for="' . $this->view->escape($id) . '"'; + } + else { + $for = ''; + unset($attribs['disableFor']); //Remove $attribs['disableFor'] from array + }

     // enabled; display label
     $xhtml = '<label'

Comments

Posted by Dolf Schimmel (Freeaqingme) (freak) on 2009-11-09T17:24:11.000+0000

What is the outcome you had expected, and how do you propose to solve this (without losing functionality, while generating valid html for other versions)?

Posted by Ernest Szulikowski (est) on 2009-11-09T17:41:07.000+0000

The expected outcome is just

test, instead of test

as attribute 'disabledfor' is not allowed in any of HTML (and XHTML) flavors.

Posted by Ernest Szulikowski (est) on 2009-11-09T17:43:58.000+0000

I think the expected behaviour was meant to be like commented by Mr O'Phinney:

http://framework.zend.com/issues/browse/…

Posted by Ernest Szulikowski (est) on 2009-11-10T06:29:51.000+0000

I found problem in Zend_View_Helper_FormLabel.

If 'disableFor' attribute is set (and for radio it is) $for is cleaned to empty string, but disableFor attribute still exists $attribs array, so $this->_htmlAttribs($attribs) produces all attribs for label including disablefor="1". The simple solution could be just to remove array element:

// $for = (empty($attribs['disableFor']) || !$attribs['disableFor']) // ? ' for="' . $this->view->escape($id) . '"' // : '';

    if (empty($attribs['disableFor']) || !$attribs['disableFor']) {
        $for = ' for="' . $this->view->escape($id) . '"';
    }
    else {
        $for = '';
        //Remove $attribs['disableFor'] from array
        unset($attribs['disableFor']);
    }

Posted by Ernest Szulikowski (est) on 2009-11-10T06:32:22.000+0000

if (empty($attribs['disableFor']) || !$attribs['disableFor']) { $for = ' for="' . $this->view->escape($id) . '"'; } else { $for = ''; unset($attribs['disableFor']); }

Posted by Ernest Szulikowski (est) on 2009-11-10T06:42:16.000+0000

I found problem in Zend_View_Helper_FormLabel.

If 'disableFor' attribute is set (and for radio it is) $for is cleaned to empty string, but disableFor attribute still exists $attribs array, so $this->_htmlAttribs($attribs) produces all attribs for label including disablefor="1". The simple solution could be just to remove array element:

// $for = (empty($attribs['disableFor']) || !$attribs['disableFor']) // ? ' for="' . $this->view->escape($id) . '"' // : '';

    if (empty($attribs['disableFor']) || !$attribs['disableFor']) {
        $for = ' for="' . $this->view->escape($id) . '"';
    }
    else {
        $for = '';
        //Remove $attribs['disableFor'] from array
        unset($attribs['disableFor']);
    }

Posted by Ernest Szulikowski (est) on 2009-11-11T02:03:27.000+0000

Sorry for double post.

Anyone can confirm this ?

What shall I do now ?

Posted by Mon Zafra (monzee) on 2009-11-19T21:23:53.000+0000

Added code to unset 'disableFor' attribute if set, attached the patch and test. Please review.

Posted by Mon Zafra (monzee) on 2009-11-19T22:20:11.000+0000

Changed diffs to svn-generated diffs instead of Netbeans.

Posted by Craig Slusher (sleek) on 2009-11-19T22:41:28.000+0000

Accidentally assigned to myself.

Posted by Matthew Weier O'Phinney (matthew) on 2009-11-20T11:19:45.000+0000

Patches applied to trunk and 1.9 release branch -- thanks!