content_extra_weights gets corrupted and breaks the ordering of form elements on node edit form

We have the same problem. On our node edit forms the fields are all mixed up after updating to Content Construction Kit (CCK) Version: 6.x-2.2

On top of the form are all the CCK-Fields. Below are the fields served by the the node module and other modules (Captcha, Printer fiendly pages and Creative-Commons Lite)

We use 8 different node types with a maximum of 11 fields. We did not add any fields after updating. Before updating they appeared where they should be. We used the http://api.drupal.org/api/function/hook_form_alter/6 to define their weights, which are set correctly but are not respected.

We could locate our problem and found a workaround.

The Problem is: The fields served by the node-module seem to have fixed weights. In the case of our story_form:

- Title-field: -5
- Language-Seletion-Field 0
- Captcha-field: 0 (old bug: http://drupal.org/node/367046)
- Body-field: 1
- Upload-field: 5

The workaround is to take this preset structure and to put the other fields around those. It's not good, but it works.

Not sure if this is the best way, but here's an updated version of that ensure all items managed by CCK (*) get a negative weight, so any other item is necessary rendered below.

(*) By "all items managed by CCK" I mean: fields, groups and elements included by hook_content_extra_fields().

/**
 * Pre-render callback to adjust weights of non-CCK fields.
 */
function content_alter_extra_weights($elements) {
  if (isset($elements['#content_extra_fields'])) {
    foreach ($elements['#content_extra_fields'] as $key => $value) {
      // Some core 'fields' use a different key in node forms and in 'view'
      // render arrays. Check we're not on a form first.
      if (!isset($elements['#build_id']) && isset($value['view']) && isset($elements[$value['view']])) {
        $elements[$value['view']]['#weight'] = $value['weight'] - 1000;
      }
      elseif (isset($elements[$key])) {
        $elements[$key]['#weight'] = $value['weight'] - 1000;
      }
    }

    if (module_exists('fieldgroup')) {
      foreach (array_keys(fieldgroup_groups($elements['#content_type'])) as $group_name) {
        if (isset($elements[$group_name]) && isset($elements[$group_name]['#weight'])) {
          $elements[$group_name]['#weight'] -= 1000;
        }
      }
    }
    $content_type = content_types($elements['#content_type']);
    foreach (array_keys($content_type['fields']) as $field_name) {
      if (isset($elements[$field_name]) && isset($elements[$field_name]['#weight'])) {
        $elements[$group_name]['#weight'] -= 1000;
      }
    }

    // Make sure this process is not executed more than once.
    // Note that the node form is rendered twice. Once from FAPI and a second time
    // from theme_node_form() in node.pages.inc.
    unset($elements['#content_extra_fields']);
  }
  return $elements;
}

In content_nodeapi() we need to apply:

     if ($op == 'view') {
       $node->content['#pre_render'][] = 'content_alter_extra_weights';
       $node->content['#content_extra_fields'] = $type['extra'];
+      $node->content['#content_type'] = $node->type;
     }

And in content_form_alter():

     }
     $form['#pre_render'][] = 'content_alter_extra_weights';
     $form['#content_extra_fields'] = $type['extra'];
+    $form['#content_type'] = $form['#node']->type;
   }

At least, I believe something like this would provide a base, so that anything managed by CCK gets a negative weight.

I don't think negative weights for cck-managed items is the solution. At least it's not a solution for us. We want to be able to arbitrary set up the fields. I finished the workaround mentioned above, but found out that the node-module-managed fields have different (fixed) weights in different node-types.

In our case:

title -5 | -5 | -5 | -5
language 0 | 0 | -2 | -1
body -1 | -1 | -1 | -0
upload 5 | 3 | 0 | 4

The problem is that CCK reorders all top level element in the form it knows about, but you do not know where they will be until CCK's #pre_render handler is executed. drupal_render() will reorder the elements after all #pre_render callbacks have been executed.

CCK deals with fields, groups and elements that are exposed to CCK though hook_content_extra_fields(), and all these elements can be reordered by the user from the 'Manage Fields' panel of each content type. In that panel, the drag'n'drop operations will compute the weights of elements dynamically, so the resulting weight values are unknown. There's no real control of the bounds that applied to weight values.

As a result of this, elements added to the form that are unknown to this CCK behavior may end up in any position in the form.

For example, after reordering fields in CCK, we may get the following weights: -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, ... 17, 18, 19, 20, 21, ... an so on, depending on the number of top level elements. Note that the actual initial weight could start at any value.

Now, if you add stuff in the form using hook_form_alter() with a weight of 10, it will be rendered in the middle of CCK fields. But it may really appear in any position, depending on the weights assigned by drag'n'drop operations and the number of top level elements CCK manages.

The patch I posted would move all CCK managed weights high enough, so that they always appear on top of any other form element.

One thing you could do is implement hook_content_extra_fields() and your stuff will be managed by CCK, so the user can move your elements in 'Manage Fields' panel. Or you could use a weight that's low enough to stay below CCK managed stuff. But if nothing is done, then you never know where your stuff will be rendered unless you implement hook_content_extra_fields(), or dynamically compute your weight adding a #pre_render handler that's executed after the one that CCK performs.

The problem with that solution is that there is then no possible way to move fields to the bottom of the page. I ran into a related problem when using the Vertical Tabs module where it turns out that re-arranging form elements has no effect in vertical tabs. That, in turn, is because Vertical Tabs is rearranging the elements during hook_form_alter, before CCK has changed the values.

It seems to me that there is no reason why CCK cannot set the weights during hook_form_alter instead of waiting until pre_render. I tried making that change and it appears that everything will work correctly if I do that. I don't know what, if any, effect it has on multigroup to do it this way, and yched set up the original method so I'd want him to take a look to make sure I'm not missing anything obvious by making this change, but for me this works.

While I was at it I added in settings for the other top-level node form elements, revision_information, author, and options, so you can re-arrange them along with the CCK fields. Any non-core, non-CCK fields will still have to expose themselves to CCK if they want to be re-orderable.

So here's my patch for yched and markus to try out. If it seems OK, it will make it possible to re-order form elements like the node author field and have the re-ordering work correctly in Vertical Tabs.

The related issue in Vertical Tabs is #444378: Fields re-ordered by CCK do not have the right weight in Vertical Tabs. Also note that we certainly can alter the weights of non-CCK fields earlier, the only question is when to set the weights for CCK fields. If this patch is a problem, we could set the weight of non-CCK fields during hook_form_alter and do the CCK fields during pre_render.

@Karen: I think there's an additional problem, which comes from the fact that after the "Manage fields" is submitted, a separate module trying to add stuff to the node form or view does not have a reference to know where fields start or end, where the body, or the administrative fieldsets of the node. So the weight of elements added by someone that does not implement hook_content_extra_fields() may end up "anywhere".

To solved this on other modules, I'm using something like this:

function mymodule_content_extra_fields($nodetype) {
  $extra = array();
  $extra['myelement'] = array(
    'label' => t('My element label'),
    'description' => t('...'),
    'weight' => mymodule_get_element_weight($nodetype),
  );
  return $extra;
}

// Generic function to get the highest element weight for the given content type.
function mymodule_get_element_weight($nodetype) {
  $weights = array();
  if (module_exists('fieldgroup')) {
    foreach (fieldgroup_groups($nodetype) as $group) {
      $weights[] = $group['weight'];
    }
  }
  $content_type = content_types($nodetype);
  foreach ($content_type['fields'] as $field) {
    $weights[] = $field['widget']['weight'];
  }
  return !empty($weights) ? (max($weights) + 1) : 1;
}

That's not perfect either, but maybe CCK could provide an API to get the top level weight of an element (field, group or extra item) ?

Well, the original mechanism was doing everything in form_alter, and it was later changed to #pre_render in #356158: Drag and Drop field weight assignment is buggy :-)
I don't really remember what bugs this move addressed, but it seems to have fixed some...

Also, note that depending on hook order, not all contrib node additions are in place by the time cck's own hook_form_alter fires.

Ugh! OK, well if we leave this only in pre_render there is no obvious way for Vertical Tabs to work. Nor for any other module that wants to use hook_form_alter(). And Vertical Tabs has already set itself to a system weight of 300 (!) to be sure it is the last one to touch the form.

But of course it is true that we don't have all contrib modules by this time either, so we can't set all weights at this point. We could reset CCK's weight to a higher number, but changing that could break some other module that depends on running after CCK (like fieldgroup).

So one option could be to create a helper module (CCK weights) with a system weight of 299 that would do nothing but form_alter the extra weights after everything has been added to the form. We would leave the original code in place in CCK so everything will work as it does now without it and still set weights in pre_render. The helper module would just ensure that they also get set earlier, in form_alter. And maybe this helper module could also provide some API's for getting information about weights of various elements.

This helper module would not be needed for normal processing, so you could operate without it and enable it only if you need finer control over weights, like when using Vertical Tabs.

It would probably be a pretty simple little module. Does that make any sense or do you see a better solution?

I believe a separate module will confuse a little.

Karen, your patch in #8 includes support for a few more extra elements. That seems to me a good move. It reduces the chances that some elements end up in an unknown position. Maybe that's enough for the vertical tabs issue?

Another thing that could be done here would be to ensure all elements managed by CCK have a weight lower than a certain value, maybe a negative weight is enough as I tried with the patch in #5.

Other modules adding elements to the node edit form would have to implement hook_content_extra_fields() themselves. And that should cover all possible cases.

Other than that, I believe I don't clearly see which is the exact problem we're trying to solve here. :-/

There are several problems here, but the particular one I was trying to address was the fact that the form weights are hard for users and other modules to control. CCK has completely taken them over in a fairly aggressive way. The hook_content_extra_fields is great for adding *new* fields to the form, but does nothing for modules that want to rearrange *existing* fields.

Vertical Tabs is not adding elements to the form, it comes in during hook_form_alter and pulls out the elements that belong in vertical tabs (node fields like the menu widget and the author widget) to move them into a separate fieldset that will drop down to the bottom of the form and show up in vertical tabs. If the weights are not set to the right values at that point (and they are not), they go into the vertical tabs fieldset with the wrong values (the unaltered, original weights), and then 'disappear' from the form and never get reset. Adding the new elements to the Manage fields screen is a nice addition, but it won't fix that problem, and changing the weight of CCK fields won't do anything either.

At the moment, I see no way for other Vertical Tabs to come in after CCK and move those elements, with the correct weights, into its fieldset, since the weights are set so late in the process. I was trying to find a way to set the weights during hook_form_alter, figuring it would also generally be helpful for other modules that want to manipulate the node form.

The only other options is for Vertical Tabs to rework its processing so it does something using pre_render, and it will have to be sure that it is done so that it happens after CCK and Fieldgroup's pre_render and before any of the child elements are actually rendered. Having working a lot with FAPI, I think that will be difficult to do. Diving in at a particular point during the render process is pretty tricky.

I guess the last ditch other possibility is for Vertical Tabs to get the form weights directly in the same way CCK gets them and set them itself. But that seems pretty crufty.

I possibly should have started a separate issue for this, but it's all related to the way we handle these extra field weights, so I threw it in here. And Vertical Tabs is in core for D7, so I figured it was worth spending some time to get it working right in D6 (even though the D7 workflow will be different).

Or we add a hook_content_extra_fieldset to add a fieldgroup to the Manage fields screen so people can drag and drop fields directly into the Vertical Tabs fieldset and arrange them there, then convince the Vertical Tabs maintainers to support that alternative :/

There are several problems here, but the particular one I was trying to address was the fact that the form weights are hard for users and other modules to control. CCK has completely taken them over in a fairly aggressive way. The hook_content_extra_fields is great for adding *new* fields to the form, but does nothing for modules that want to rearrange *existing* fields.

Thanks for the clarification.

Well, here's how I see why CCK was changed to do this process at #pre_render time rather than directly in hook_form_alter().

1. The form is built by the node module, which provides core element with known weights.
2. Other core and contrib modules (including CCK) should implement hook_form_alter() against the node edit form to *insert* new elements.
3. CCK wants to provide a method for users to rearrange fields, but also some other elements, provided they are exposed via hook_content_extra_fields().
4. When CCK is installed, it rebuilds the weight of elements (fields, groups and those exposed extra elements), but the weight values themselves after this process is *unknown* (no bounds are forced for the resulting weights). Potential problem: any other top level element in the node edit form not exposed via hook_content_extra_fields() may end up at any location. I'm afraid there's nothing else CCK can do here. Such modules must implement hook_content_extra_fields(). CCK already exposes core elements provided by taxonomy module, etc. Contrib should do it themselves in order to integrate well with CCK.

Now, even when modules expose form elements via hook_content_extra_fields(), these modules may be executed after CCK. That means the hook_form_alter() implementation of CCK may not have all the elements it has been told to manage in place, just because other modules need to use their own implementation of hook_form_alter(), and depending on the module weight itself, they may be executed before CCK, but also after CCK. This is why CCK cannot apply weights at hook_form_alter() time. Here's the related issue mentioned earlier by yched where this fact was identified: #356158: Drag and Drop field weight assignment is buggy

Since that issue was fixed, CCK now applies weights at #pre_render time, where all other modules that have exposed form elements have had the chance to insert them in the form, so CCK can happily proceed.

Now the problem with modules such as Vertical Tabs is that they may need to rearrange form elements, but they must do it *after* CCK has done its job.

As I see it, this leads us to a couple of questions:

a) Is it possible for CCK to rearrange elements earlier, somewhere between hook_form_alter() and pre_render?
b) Is it possible that other modules such as Vertical Tabs may inject their own code to do their job after CCK has rearranged elements?

One possible answer to a) is that CCK could rebuild the weights during a new after_build callback. But, is it really necessary when b) is possible?

To answer b), I think modules such as Vertical Tabs need to alter their weight so they execute after CCK, and then they can append their own pre_render callback, so they have the chance to rearrange elements as they wish. Though, they may need to do part of their job during hook_form_later().

If the answer to b) is no, such modules need to do some kind of job during the form build process and before the form rendering process. Then, we may try to implement CCK weights alterations using a specialized after_build callback. And then, such modules have their chance adding their own after_build callback to be executed after the one provided by CCK.

I haven't really tried vertical tabs, so I cannot really tell. Maybe I'm missing something, but I believe there's little margin to resolve these issues due to the complexity of the relations and dependencies of all processes involved here. I guess this complexity is something that will also affect D7.

OK, I am trying to solve this with a patch to Vertical Tabs which hopefully will be accepted. They can't use pre-render because they're adding javascript elements to the form, so they'll have to use content_extra_field_weight() to find the right values. We'll have to watch to see if this is needed in the D7 version, I don't know if it is or not.

And I will also go ahead and commit the code to add additional core elements to CCK for re-ordering in the CCK UI.

Head-spinning issue :-/

I don't think I see another way for 3rd party modules acting on node forms to use CCK's extra_fields features. Fields drag-n-drop reordering indeed make weights unpredictable. Unfortunate and fairly rude for sure, but people value the feature - and, er, i don't think there's any going back :-p.

tabledrag.js supports two types of FAPI element for the hidden 'weight' input : #type = weight (select from -X to +X) and #type = textfield. We currently use the latter.

I think with type 'textfield', actual weight values are redefined from 0 when a row is dragged, while with type 'weight selector', they are redefined from the lowest allowed -X, which probably adds even more 'uncertainty' on the final weights that get saved.
This tabledrag behavior can be seen by removing the tabledrag 'hide weight inputs' feature. template_preprocess_content_field_overview_form() has ready-to-uncomment replacements for the drupal_add_tabledrag() calls just to this effect.

I don't know if moving to textfield for our weights would remove enough uncertainty to make the change valuable. Not sure either how this would impact already saved field orders.

Doh, scratch that. We already use 'type = textfield' for weight selectors on the field overview pages. So, no room for improvement.

Follow up to address a few issues:

- Adding 'author' and 'options' has no effect. The node module renders these elements at the end of the form, just before the 'buttons' element. For reference, see theme_node_form() in node.pages.inc

- The comment module element is *always* present when comment module is enabled, despite the value of the comments option to allow users with 'administer comments' permission change the option at node level at any time.

- Adding extra elements for path and translation settings.

Looks good to me.

Committed patch in #20 to CVS, against the branches for CCK2 and CCK3. Thank you, Yves, for checking. :)

Now, I believe CCK itself covers all extra elements present in core. Contrib modules adding stuff to the node edit form need to implement hook_content_extra_fields() to expose their elements, so that the user can drag them to wherever they see fit.

I seem to have the exact problem described in the original issue.
One one particular content type, I have absolutely non control on the body field.
I can always move it to the top of the form by dragging the Title (not the Bod filed) to the top.
Then, If I move the body down (because I want to insert cck field between the Title and the body), it goes randomly wherever it wants, pulling the title with it (they mostly stick to each over, but wherever is the title, it always display on the top).
I play this game hundreds times until it the body reaches the desired place...

I tried to change the values in content_extra_weight, but this did not help.

Reading this post, I am not sure if the problem was actually solved and if I suffer of this issue or another one. (running cck3-alpha 3)

Thanks for help