Send generates invalid code
suit4 - September 10, 2007 - 11:06
| Project: | Send |
| Version: | HEAD |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | jerdavis |
| Status: | closed |
Description
The send form contains invalid code.
There is no attribute like 'clear="all"', better replace with something like 'class="clear_all"'
The form id is truncated to '#-send-form', which does not validate.
#1
According to http://www.drupalcenter.de/node/8001 (German only), #-send-form is problematic in Internet Explorer 6. The proposed solution is to rename the function _send_form to send_form.
#2
Created a patch to remove preceding _ from send_form* functions in order to correct the issue with the invalid -send-form ID.
#3
The issue here is that by renaming the form id, you'll invalidate the use of any form_altering that may be affecting send forms.
For example, the News module alters forms with an id of _send_form, and this functionality would break. This is also going to affect any site-specific form behavior overrides.
#4
If a new version of a module breaks other modules you have installed, you should not upgrade. Is there some way to specify compatibility in *.info files? If not, a proper release note should do the trick. If the solution outlined in #2 is committed, I will look for dependent modules that get broken.
It is easier to change those modules to become compatible with a new version of the Send module, than to change the XHTML specification in a way to make the generated code valid :-)
#5
That's a fair point.
However, this is less about other contrib modules, and more about the likelihood that someone has written a form_alter hook locally to their site. For example, to support multiple recipients, you can form_alter the _send_form to unset the 'recipient_*_name' fields and add a 'recipients' field.
If you have done that - or anything similar - updating to this patch would revert all of your forms. For that reason, this patch needs to be in a separate release.
Since we haven't yet got an official D5 release for send, we'll be backporting changes from HEAD and rolling one. As it breaks compatibility, we will then roll an additional release that notates the change.
#6
Just committed a fix for this to HEAD. I'm changing the #id value on the form to "send_form", which will remove the preceding _ from the css ID
This may create regression for CSS/theme work, but will keep things API-portable for other modules and their form_alters
#7
So, this is fixed, no?
#8
Automatically closed -- issue fixed for two weeks with no activity.