#prefix is rendered inside form tags when a theme_form_id function exists.

To reproduce: add a #prefix to the node submission form, view source and observe that it is rendered inside the form tag. Next, remove / rename theme_node_form and observe that the prefix is correctly rendered outside the form tag.

CommentFileSizeAuthor
#12 form_inc.patch1.53 KBchx
#7 drupal_render_theme_prefix.patch1.41 KBchx
#2 exp.patch.txt978 bytesheine

Comments

heine’s picture

heine commented

Note: it doesn't just happen to the node submission form; try the following with and without theme_test_form().

function test_form() {
  $form['#prefix'] = 'testprefix';
  return $form;
}

function theme_test_form($form) {
  return drupal_render($form);
}
heine’s picture

heine commented
StatusFileSize
new exp.patch.txt978 bytes

Experimental, so no 'needs review'. Patch takes the approach that prefixes cannot be touched by the elements theme function.

anders.fajerson’s picture

anders.fajerson commented
Status: Active » Needs review

This indeed makes the patch in http://drupal.org/node/100775 work. Can't really comment on the implementation, but I'm tempted to set this as critical.

anders.fajerson’s picture

anders.fajerson commented
Status: Needs review » Active

Sorry, missed the "no" in "so no 'needs review'.

heine’s picture

heine commented

Apart from having an inconsistent and broken implementation, consequences of this bug are:

  1. Node previews containing forms will not work (see seperate http://drupal.org/node/100775).
  2. Previewing comments on form containing nodes (such as polls) will give a validation error but *only* when a theme function for comment_form is present.
heine’s picture

heine commented
Title: #prefix rendered inside form tags when theme function exists » having a #theme for an element changes the meaning of prefix from "prefix of this element" to "prefix of the children elements"
Priority: Normal » Critical
chx’s picture

chx commented
Assigned: Unassigned » chx
Status: Active » Needs review
StatusFileSize
new drupal_render_theme_prefix.patch1.41 KB
heine’s picture

heine commented
Status: Needs review » Reviewed & tested by the community

#7 is much more elegant. This patch (or similar) needs to go into Drupal 5 because our javascript may break when divs are moving around when someone creates a theme_form_id override (upload, collapse).

#7 fixes the issue and did not introduce other problems during testing.

dries’s picture

dries commented

It's hard to understand/grok that change. What exactly is going on? Inheritance? And can we document that in the code?

dries’s picture

dries commented
Version: 5.x-dev » 4.7.x-dev

Committed to CVS HEAD.

I think this needs to be backported so changing the version. :)

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Reviewed & tested by the community » Patch (to be ported)

chx will re-roll later

chx’s picture

chx commented
Status: Patch (to be ported) » Reviewed & tested by the community
StatusFileSize
new form_inc.patch1.53 KB

Indeed he will.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Reviewed & tested by the community » Fixed

applied

dww’s picture

dww
we/he/they
commented
Assigned: chx » dww
Category: bug » task
Priority: Critical » Normal
Status: Fixed » Active

FYI: the only "downside" of this patch is that modules which used to use #prefix to add a div to their node form used to see the divs in this order: .node-form .custom. now, it's reversed: .custom .node-form. this has been screwing up the CSS for the project_release [1] and project_issue [2] UI. not the end of the world, but just thought i'd mention it. ;) i already documented this at http://drupal.org/node/64279#prefix when i ran into it during the 5.x port of project*, but it appears this is now an issue for 4.7.4 -> 4.7.5 porting, too. should we add such a page? anything else change between 4.7.4 and 4.7.5 that needs documentation like this?

thanks,
-derek

[1] http://drupal.org/node/105228
[2] http://drupal.org/node/105229

dww’s picture

dww
we/he/they
commented
Assigned: dww » Unassigned
Category: task » bug
Priority: Normal » Critical
Status: Active » Fixed

http://drupal.org/node/105960 is now there.

setting everything back for posterity.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)