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.

CommentFileSizeAuthor
#2 send_form_remove_underscore.patch1.57 KBjerdavis
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

traxer’s picture

The form id is truncated to '#-send-form', which does not validate.

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.

jerdavis’s picture

Version: 5.x-1.x-dev » master
Assigned: Unassigned » jerdavis
Status: Active » Reviewed & tested by the community
FileSize
1.57 KB

Created a patch to remove preceding _ from send_form* functions in order to correct the issue with the invalid -send-form ID.

Allie Micka’s picture

Status: Reviewed & tested by the community » Needs work

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.

traxer’s picture

Status: Needs work » Needs review

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 :-)

Allie Micka’s picture

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.

Allie Micka’s picture

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

dww’s picture

Status: Needs review » Fixed

So, this is fixed, no?

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.