Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
node system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
27 Oct 2008 at 04:44 UTC
Updated:
20 Jan 2009 at 20:40 UTC
Jump to comment: Most recent file
Comments
Comment #2
lilou commentedI confirm, node-form appear twice (id & class) :
Comment #3
lilou commentedAttached patch, rename form
id="node-form"toid="node-form-article".Comment #5
lilou commentedSimilar issue : #276653: Duplicate element ID renders invalid HTML
Comment #6
dries commentedWell, it seems like on my copy, the form-tag is opened twice. See attached screenshot.
Comment #7
riccardoR commentedThe form-tag is opened twice actually. This also happens in many (maybe all) forms rendered with a theming function defined in $form['#theme'].
See for example admin/content/taxonomy and admin/user/user (two duplicate forms here).
This doesn't happen on D6.8
The difference to HEAD is in drupal_render() :
On D6 the variables #type, #prefix and #suffix are saved and unset before calling the custom #theme function - which ends calling drupal_render() again - and restored afterwards.
drupal_render() itself takes care of those items after all other have been processed.
Well, the properties are saved and restored on HEAD as well, but #type is no longer unset in the meanwhile.
In drupal_render() - D6.8 we have:
while in D7-HEAD we have :
This was changed here: http://drupal.org/node/252013 (removed save, unset, restore of all three variables)
and partially restored here: http://drupal.org/node/295506 (except the unset of the #type variable, probably because the problem was less noticeable)
IMO it makes sense to unset the #type property as it is already saved and restored, and it is rendered correctly at the end of drupal_render() as in D6.
The attached patch fixes all duplicate form-tags I was able to find and test, and apparently it has no side effects.
Comment #8
wrwrwr commentedNo sure what the implications of unsetting #type are, but the form is opened just once and the page validates with the patch.
Comment #9
dries commentedI'm wondering if we could add a XHTML validation step to D7's test framework so that every page that is tested automatically gets validated ...
Either way, I'm running all the tests with this patch, to see if they succeed.
Comment #10
dries commentedAll tests pass. Committed to CVS HEAD. Thanks.
Comment #11
catchXHTML validation is an interesting idea - started a new feature request against simpletest here #355185: Add HTML validation to functional tests