unnecessary divs inside the form theme_form()
mortendk - June 18, 2009 - 17:10
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | markup |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | d4d, Favorite-of-Dries, markupmarines |
Jump to:
Description
This might be a small trivial thing but...
<?php
function theme_form($element) {
// Anonymous div to satisfy XHTML compliance.
$action = $element['#action'] ? 'action="' . check_url($element['#action']) . '" ' : '';
return '<form ' . $action . ' accept-charset="UTF-8" method="' . $element['#method'] . '" id="' . $element['#id'] . '"' . drupal_attributes($element['#attributes']) . ">\n<div>" . $element['#children'] . "\n</div></form>\n";
}
?>I cant see why were having a div inside the form - both are block elements (http://www.w3.org/TR/html4/sgml/dtd.html#block))
now i know a form is only allowed to have stuff in it thats a block element or a script:
http://www.w3.org/TR/html401/interact/forms.html#h-17.3:
BUT all form elements that drupal is putting into forms surrounded by an div so that makes this div unnecessary.
removing it would mean cleaner markup:
so from
....
....
<?php
function theme_form($element) {
// Anonymous div to satisfy XHTML compliance.
$action = $element['#action'] ? 'action="' . check_url($element['#action']) . '" ' : '';
return '<form ' . $action . ' accept-charset="UTF-8" method="' . $element['#method'] . '" id="' . $element['#id'] . '"' . drupal_attributes($element['#attributes']) . ">\n" . $element['#children'] . "\n</form>\n";
}
?>
#1
and just to correct the code ;)
<?phpfunction theme_form($element) {
$action = $element['#action'] ? 'action="' . check_url($element['#action']) . '" ' : '';
return '<form ' . $action . ' accept-charset="UTF-8" method="' . $element['#method'] . '" id="' . $element['#id'] . '"' . drupal_attributes($element['#attributes']) . ">\n" . $element['#children'] . "\n</form>\n";
}
?>
so from:
<form><div>
.....
</div>
</form>
to:
<form>.....
</form>
#2
I am in full support of this removal. Seems like it had good intentions at the time, but has just created mess in time.
#3
Can we verify that validator.w3c.org still likes us if this change is made?
#4
its http://validator.w3.org and yes have checked it but just to be sure ill set up a huge test for it :)
#5
hmm looks like everything else than buttons are packed inside divs or other block elements.
one could argue that buttons should be themed as other form elements
but unless buttons are packed inside a block element it woudnt be a good idea to remove the div :/
#6
Look into CVS history to learn why these DIV containers are required.
#7
Aroo? Just how far back does one have to look?
#8
It has been like that since the first checkout of form.inc in Drupal CVS. As far as I understand, this is not necessary (anymore).
#9
For putting buttons in a block element, there's #482816: Make a consistent wrapper around submit buttons which I think will do what you mean?
#10
Tagging!
#11
it needs 2 things:
A wrapper around the buttons (submit, button, cancel, image ) + a wrapper around hidden fields.
it would be an huuuge improvement if we could collect the submit, delete etc buttons in a wrapper - cause aligning all the buttons to the right today is a pain in the neck, and involes all kinds of nasty float:right trix
The hidden fields are afaik always added at the bottom at the form they could get collected in their own little .... ?
Guess we still have to figure out what to do with buttons that is put inside a form but a simple would do the trick inside theme_button & theme_image_button ?
#12
I have a gut feeling that div is there for a reason, probably to deal with various IE layout bugs, for instance if you wanted margin/padding/borders on the form and to center it...I think there's also an IE6 bug where any styling applied to the form tag results in white space being added to the bottom of the form?
But, IE6 is sliding slowly into irrelevance and similar affects can probably be achieved with wrappers on elements.
So:
+1 to a wrapper around the buttons
But this change will need some serious testing in IE, methinks.
#13
I never had any issues to apply different, advanced form layouts to Drupal's output. And I did a lot in the past. But most often sticking to a two-column form layout (where label appears left and the field right next to it).
I fear that removing the wrapper-DIV adds a lot of unnecessary burden to the rendering of form elements - while making Drupal's output potentially XHTML-invalid or deferring penalty to themers that need to support IE properly.
Won't fix for me.
#14
That empty div has driven me crazy for years! We should at the very least give it a class so that we have some context to it...