This originally was reported in the D6 backport of vertical tabs, but confirmed a major bug in D7 core as well. If a fieldset has set a #prefix and #suffix value (like file.module), things go horribly, horribly wrong. For instance, here's comment.module's fieldset that it adds to the node add/edit forms:

<?php
function comment_form_alter(&$form, $form_state, $form_id) {
  if (!empty(
$form['#node_edit_form'])) {
   
$node = $form['#node'];
   
$form['comment_settings'] = array(
     
'#type' => 'fieldset',
     
'#access' => user_access('administer comments'),
     
'#title' => t('Comment settings'),
     
'#collapsible' => TRUE,
     
'#collapsed' => TRUE,
     
'#group' => 'additional_settings',
     
'#attached' => array(
       
'js' => array(drupal_get_path('module', 'comment') . '/comment-node-form.js'),
       ),
     
'#weight' => 30,
    );
?>

If I add the following to the fieldset's definition:

      '#prefix' => '<div class="comment-fieldset">',
      '#suffix' => '</div>',

Resulting screenshots pending...

Files: 
CommentFileSizeAuthor
#15 drupal7-vertical-tabs-fieldset-fix-update2.diff10.9 KBarithmetric
PASSED: [[SimpleTest]]: [MySQL] 20,426 pass(es).
[ View ]
#9 attachment1.png68.94 KBcosmicdreams
#9 attachment2.png49.28 KBcosmicdreams
#8 drupal7-vertical-tabs-fieldset-fix.diff8.65 KBarithmetric
PASSED: [[SimpleTest]]: [MySQL] 17,537 pass(es).
[ View ]
#1 screenshot_009.png40.19 KBDave Reid
#1 screenshot_010.png34.62 KBDave Reid

Comments

Dave Reid’s picture

StatusFileSize
new34.62 KB
new40.19 KB

Result is that the comment fieldset ended up in *all* of the vertical tabs fieldsets.

jim0203’s picture

subscribe

dmitrig01’s picture

Wow, this is a major WTF

Dave Reid’s picture

It gets even better since AJAX add-more forms rely on these surrounding prefix/suffix markup to attach its 'more' items. And so they fail miserably as well.

Dave Reid’s picture

This is also a big problem with update.module in D6 with the backport of vertical tabs.

bwynants’s picture

subscribe

bomarmonk’s picture

Subscribe... hoping for a backported fix to Drupal 6.

arithmetric’s picture

Status:Active» Needs review
StatusFileSize
new8.65 KB
PASSED: [[SimpleTest]]: [MySQL] 17,537 pass(es).
[ View ]

Attached is my patch for Drupal 7 to resolve this issue.

My approach is to wrap fieldsets grouped by vertical tabs with a <div> tag so that fieldsets with #prefix and #suffix content is supported and displayed within the tab content area.

To accomplish this, I add a new theme function (theme_vertical_tabs_fieldset) that is assigned as a theme wrapper function for all fieldset children of a vertical tab group.

I'd appreciate others' review of this patch.

cosmicdreams’s picture

Status:Needs review» Needs work
StatusFileSize
new49.28 KB
new68.94 KB

I'm trying this patch out today on a fresh cvs install:

Before patch : attachment1.png
After patch: attachment2.png : Tested with Chrome and Firefox

So, the patch didn't seem to work. Instead the entire Vertical menu becomes broken and. I'm switching to needs work.

casey’s picture

The class "fieldset-wrapper" is being used inside fieldsets since #676800: Fieldsets break design badly so you can't use it here any more.

Can't we just remove the #prefix and #suffix from all child elements in form_process_vertical_tabs()?

Bojhan’s picture

@davereid Can you review the patch ?

arithmetric’s picture

I'll also take another look at the patch. It looks like the CSS to compensate for the extra <div> may not be quite right. However, I don't think this problem was present when using the overlay module.

In any case, the rendering of the vertical tabs needs to be compared before and after patching.

sivaji@knackforge.com’s picture

Vertical tabs looks fine for me with recent code base from HEAD see
http://o.imm.io/kRd.png
http://o.imm.io/kRe.png
http://o.imm.io/kRf.png

If anyone is able to reproduce this bug should take a look at #755030: Regression: #field_prefix/suffix is displayed before the label as well.

moshe weitzman’s picture

Priority:Critical» Normal

This bug may be fixed according to most recent post and does not sound like "my site is crippled" critical. Still a bug, but downgrading priority

arithmetric’s picture

Status:Needs work» Needs review
StatusFileSize
new10.9 KB
PASSED: [[SimpleTest]]: [MySQL] 20,426 pass(es).
[ View ]

This issue is still present, but it only shows up when a fieldset with #prefix or #suffix content is rendered within vertical tabs. I'm not sure if this happens in the Drupal core code, so it may not be a critical issue for most users.

In any case, here's an update to my patch from #8 with a few minor changes. Among other things, I replaced the use of the "fieldset-wrapper" class, as @casey suggested in #10, and I fixed a bug in the previous patch related to the spacing and layout, which may relate to what @cosmicdreams noted in #9.

tim.plunkett’s picture

subscribe

Renee S’s picture

subscribe

DamienMcKenna’s picture

Am running into this too :-\

DamienMcKenna’s picture

Tried patching #15 against D7 (snapshot from 201008062300) and other than some offsets it patched just fine. I haven't *tested* it yet, will try to do so this weekend.

sun’s picture

Status:Needs review» Needs work

Introducing a new #theme and #pre_render functions that do (almost) the same as the existing ones is not the right solution.

For one, I wonder whether #prefix and #suffix can remotely be supported here. They can contain literally anything. But it may be possible by implementing an ugly hack.

Second, I guess we're going to touch the current implementation anyway over in #857124: Collapsible fieldsets and vertical tabs do not work without form_builder() (Form API), so the entire current #process and #pre_render combination will likely change either way.

mansspams’s picture

Subscribing

kndr’s picture

First, check this patch for D6 http://drupal.org/node/533720#comment-4293170 Maybe it is related.