search

laminas / laminas-form

Validate and display simple and complex forms, casting forms to business objects and vice versa
https://docs.laminas.dev/laminas-form/
BSD 3-Clause "New" or "Revised" License
80 stars 52 forks source link

Added novalidate to boolean attributes array #273

Closed krytenuk closed 2 months ago

krytenuk commented 2 months ago

I have added novalidate to the AbstractHelper::$booleanAttributes array as this is a boolean attribute in HTML 5. Currently setting the Element::setAttribute('novalidate', true) in the form when the doc-type is set to HTML5, renders novalidate="1" rather than just novalidate. https://html.spec.whatwg.org/#attr-fs-novalidate

gsteel commented 2 months ago

@krytenuk - The novalidate attribute is only applicable to <form> elements and the view helper for forms already has the novalidate attribute listed:

https://github.com/laminas/laminas-form/blob/3.20.x/src/View/Helper/Form.php#L28-L37

krytenuk commented 2 months ago

@gsteel, thank you for your response. Yes the attribute is set in the form view helper but not as a boolean value, the only place to currently set this is in the abstract view helper. This Without specifying the novalidate attribute as a boolean attribute, https://github.com/krytenuk/laminas-form/blob/fix/add-novalidate-to-boolean-attributes/src/View/Helper/AbstractHelper.php#L62, it does not render correctly. The boolean attribute's check and rendering was implemented in https://github.com/laminas/laminas-form/pull/108.

Before implementing my fix, setting the novalidate attribute to true, Element::setAttribute('novalidate', true), renders, novalidate="1". And setting the attribute to false, Element::setAttribute('novalidate', false), renders, novalidate="". Also setting the attribute to null Element::setAttribute('novalidate', null) or an empty string, Element::setAttribute('novalidate', '') renders the same novalidate="". As far as I am aware, all these are rendering incorrectly as novalidate is a boolean attribute according to https://html.spec.whatwg.org/#attr-fs-novalidate, please correct me if I am mistaken on this.

After adding the attribute to the AbstractHelper::$booleanAttributes array, the novalidate attribute renders correctly. Element::setAttribute('novalidate', true) just renders novalidate and Element::setAttribute('novalidate', false) causes the novalidate attribute not to render. I believe this should be the expected behaviour. Using truthy and falsy values also work.

The current logic will prevent the novalidate being rendered on any other form element as the check for this is done, https://github.com/krytenuk/laminas-form/blob/fix/add-novalidate-to-boolean-attributes/src/View/Helper/AbstractHelper.php#L396, using the array you highlighted above.

Slamdunk commented 2 months ago

Thank you @krytenuk