Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.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:
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...
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | drupal7-vertical-tabs-fieldset-fix-update2.diff | 10.9 KB | arithmetric |
| #9 | attachment1.png | 68.94 KB | cosmicdreams |
| #9 | attachment2.png | 49.28 KB | cosmicdreams |
| #8 | drupal7-vertical-tabs-fieldset-fix.diff | 8.65 KB | arithmetric |
| #1 | screenshot_009.png | 40.19 KB | dave reid |
Comments
Comment #1
dave reidResult is that the comment fieldset ended up in *all* of the vertical tabs fieldsets.
Comment #2
jim0203 commentedsubscribe
Comment #3
dmitrig01 commentedWow, this is a major WTF
Comment #4
dave reidIt 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.
Comment #5
dave reidThis is also a big problem with update.module in D6 with the backport of vertical tabs.
Comment #6
bwynants commentedsubscribe
Comment #7
bomarmonk commentedSubscribe... hoping for a backported fix to Drupal 6.
Comment #8
arithmetric commentedAttached 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.
Comment #9
cosmicdreams commentedI'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.
Comment #10
casey commentedThe 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()?
Comment #11
Bojhan commented@davereid Can you review the patch ?
Comment #12
arithmetric commentedI'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.
Comment #13
sivaji_ganesh_jojodae commentedVertical 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.
Comment #14
moshe weitzman commentedThis 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
Comment #15
arithmetric commentedThis 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.
Comment #16
tim.plunkettsubscribe
Comment #17
Renee S commentedsubscribe
Comment #18
damienmckennaAm running into this too :-\
Comment #19
damienmckennaTried 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.
Comment #20
sunIntroducing 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.
Comment #21
henrijs.seso commentedSubscribing
Comment #22
kndrFirst, check this patch for D6 http://drupal.org/node/533720#comment-4293170 Maybe it is related.