zendframework / zend-form

Form component from Zend Framework
BSD 3-Clause "New" or "Revised" License
69 stars 87 forks source link

formCollection View Helper doesn't render name #86

Open GeeH opened 8 years ago

GeeH commented 8 years ago

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7687 User: @gregGit Created On: 2016-03-14T16:20:39Z Updated At: 2016-04-13T07:48:07Z Body When using form view helper, the formCollection view helper is used to render fieldset. But form some reason that i don't understand this view helper doesn't set the name attribute in the fieldset (unset($attributes['name']); in the render Method). I belive name is a valid html attributes for fieldset i don't understand why ZF2 never render this tag in a fieldset, and it can be usefull to have it in the DOM.

For test create an action like this :

  $fs = new \Zend\Form\Fieldset('my-fieldset');
        $fs->setAttribute('name', 'my-fieldset');
        $fs->add(array(
            'type' => 'text',
            'name' => 'text',
            'options' => array(
                'label' => 'The Text'
            )
        ));

        $fs->add(array(
            'type' => 'text',
            'name' => 'title',
            'options' => array(
                'label' => 'Blog Title'
            )
        ));

        $form=new \Zend\Form\Form('my-form');
        $form->add($fs);
        return new ViewModel(array('form' => $form));

and just " echo $this->form($form)" in the view.

No attribute name for the fieldset is rendered.


Comment

User: @froschdesign Created On: 2016-04-13T07:48:07Z Updated At: 2016-04-13T07:48:07Z Body

I belive name is a valid html attributes for fieldset

It's only allowed in HTML5.


hschletz commented 6 years ago

Having a "name" attribute can be really useful -- it was added to the HTML5 standard for a reason. Being unable to use it is a real bummer, and the way the helper is implemented, there is no other choice than reimplementing it from scratch if the attribute is needed.

What about checking the doctype via the "Doctype" helper, or simply adding a flag?

froschdesign commented 6 years ago

This part needs to be changed

https://github.com/zendframework/zend-form/blob/4419eef6dbe9d276e7e27c6a25f022be74534959/src/View/Helper/FormCollection.php#L123-L126

And the property $validTagAttributes must be added to Zend\Form\View\Helper\FormCollection class.

protected $validTagAttributes = [
    'name' => true,
];
froschdesign commented 6 years ago

Unit test for this case can be:

public function testRenderCollectionWithNameAttributeAndDoctypeHtml5()
{
    $this->helper->setDoctype(Doctype::HTML5);

    $form = $this->getForm();
    $collection = $form->get('colors');
    $collection->setAttribute('name', 'foo');

    $markup = $this->helper->render($collection);
    $this->assertContains('<fieldset name="foo">', $markup);
}

public function testRenderCollectionWithNameAttributeAndDoctypeXhtml1()
{
    $this->helper->setDoctype(Doctype::XHTML1_STRICT);

    $form = $this->getForm();
    $collection = $form->get('colors');
    $collection->setAttribute('name', 'foo');

    $markup = $this->helper->render($collection);
    $this->assertContains('<fieldset>', $markup);
}
hschletz commented 6 years ago

createAttributesString() already removes invalid attributes, via prepareAttributes(). There should be no need to hardcode removal of the "name" attribute.

194 added methods to declare valid attributes, so we could add the "name" attribute manually if it wasn't forcefully removed.

However, I don't see the point in disallowing the attribute in the first place, other than compliance with doctypes other than HTML5. This only adds confusion when the attribute is ignored. Non-HTML5 developers may choose not to use it. Or allow/disallow it based on the doctype.

froschdesign commented 6 years ago

@hschletz

createAttributesString() already removes invalid attributes, via prepareAttributes(). There should be no need to hardcode removal of the "name" attribute.

It is included in the current code, therefore I wrote: "This part needs to be changed".

194 added methods to declare valid attributes...

Not needed here.

...compliance with doctypes other than HTML5.

This is the current behaviour of the entire component and there is no reason to change that.

michalbundyra commented 4 years ago

This repository has been closed and moved to laminas/laminas-form; a new issue has been opened at https://github.com/laminas/laminas-form/issues/42.