Custom element properties entirely override default element info properties

The same applies to form_builder(), identical code there.

I've already spent quite some time thinking about this problem, but wasn't able to come up with something that would always work. That is, because sometimes, you want to override the defaults. Most often that is even the case. But for some properties on some element types, you most likely do not want to skip the defaults, but that's just an assumption - your use-case of not keeping the defaults (without altering the global defaults) may be entirely valid.

And if that wasn't enough already, there's the additional problem of sort property value/callback order. Which #pre_render should run first? Which #process last? Yours or the default?

Given all that, I highly doubt there is viable solution for D7.

I ultimately came to the conclusion that this major problem will likely trigger a new way of doing things in D8:

Instead of the currently required

  $form['foo'] = array_merge(element_info('bar'), array(
    '#type' => 'bar',
    '#pre_render' => array('example_pre_render_bar'),
  ));

it may turn into

  $form['foo'] = element_info('bar', array(
    '#pre_render' => array('example_pre_render_bar'),
  ));

which may turn into

  $form['foo'] = drupal_element('bar', array(
    '#pre_render' => array('example_pre_render_bar'),
  ));

which may ultimately turn into

  $form
    ->add('foo')
    ->type('bar')
    ->appendPreRender('example_pre_render_bar');

I'm quite opposed to most usage of OOP, but it may be the only viable solution.

We can surely try this:

  $form['foo'] = element_info('fieldset', array(
    '#pre_render' => array('example_pre_render_bar'),
  ));

Which totally resolved the @todo I added to Field UI.

Fasten your seatbelt. #defaults_loaded!

The logic here looks good, but I'm a bit uncomfortable abusing element_info() for this. Perhaps I want a new element_merge() which calls into element_info(). We are making element_info() lie a bit since its doxygen says ' Retrieve the default properties for the defined element type.'

Sure, works for me :)

That’s better. But ...

I still get the sense that element_merge() is doing too much. It should only deal with #pre_render if thats the property we are 'obstructing'. All other properties ideally get filled in during drupal_render()/form_builder().

I think the attached patch gives an API which is a bit more readable since the array_merge is more compact. In the OP, we would do:

$build = array(
  '#markup' => 'something',
  '#pre_render' => array_merge(array('some_function'), element_info_property('markup', 'pre_render')),
);

The caller can easily change sort order easily. The only bummer here is that the dev still had to remember to do this but I see no way around that as sun describes in #3.

Hm. That would also be an option, though it looks more complex and more verbose. I'd like to know what others think. My consideration for the all-in-one merge based on a hard-coded assumption has been that I think that developers most often just want to prepend #process, #pre_render, #theme_wrappers, and #post_render -- as appending them after the defaults most often doesn't make sense. Additionally, when just setting a single key for #attributes, a developer completely overrides any defaults.

I.e., the idea was to make it as easy as possible, so everyone knows how the simple way works.

If you actually care for the default properties and need your value injected elsewhere, then I don't see a large difference form your code to this code:

$markup_info = element_info('markup');
$build = array(
  '#markup' => 'something',
  '#pre_render' => array_merge(array('some_function'), markup_info['#pre_render']),
);

Thoughts?

Yeah, there is hardly a difference except that mine is all on one line. And, there is a function which people learn about and use in this situation. I think that’s kind of important, or else folks will just never get the hang of this and make the same mistake as the OP.

OK, lets let others chime in.

Can I throw a 3rd option into the mix?

function element_merge($before, $type, $after) {
  $element = drupal_array_merge_deep($before, element_info($type), $after);
  $element['#defaults_loaded'] = TRUE;
  return $element;
}

The drupal_array_merge_deep() function is in #208611-27: Add drupal_array_merge_deep() and drupal_array_merge_deep_array() to stop drupal_add_js() from adding settings twice, and it's the "current" incarnation of what rfay's referring to in #1: it merges arrays and overrides scalars.

I'm not convinced that we can or should decide which things make sense before and which make sense after. #pre_render for 'markup' and 'link' needs to have the custom ones before, but that's more an idiosyncrasy of us using drupal_pre_render_markup() and drupal_pre_render_link() for what we should have done in #theme, but didn't in an attempt to eek out some performance optimization. Who's to say whether #process or #theme_wrappers needs to have element-specific come before or after what is needed for the type? Seems to me we need to leave the decision to the caller without biasing it in the API.

Just a suggestion. I'm willing to be convinced out of this if others are confident in sun's assumption that we usually want element-specific functions before the type ones. If we go with something like #9, where we do bias towards that order, then I think we should *also* add #10's element_info_property() function to make it easy for the caller to do something different. But if we like my "let the caller decide what to do before and what to do after" suggestion, then I don't think we need an element_info_property() function, though I wouldn't oppose adding it anyway, as it's a nice simple wrapper function that some people would enjoy using.

Also, while I totally appreciate the utility of this, I'm a little concerned about it being used too much. Right now, with defaults loaded later in the pipeline, you can have hook_form_alter() implementations change an element's #type (for example, from 'textfield' to 'date_popup'). Loading defaults during element construction takes this away to some extent. So I'm starting to lean towards #10, since that would ensure that defaults are loaded early only for the properties actually being customized.

Just to chime in, I would prefer the system did the heavy lifting, so the developer does less - e.g.

// element definition
$types['fieldset'] = array(
    '#process' => array('form_process_fieldset', 'ajax_process_form'),
    '#pre_render' => array('form_pre_render_fieldset'),
    '#theme_wrappers' => array('fieldset'),
  );


// form code
$form['foo'] = array(
    '#type' => 'fieldset',
    '#pre_render' => array('example_pre_render_bar),
  );


// would produce:
$form['foo'] = array(
    '#type' => 'fieldset',
    '#process' => array('form_process_fieldset', 'ajax_process_form'),
    '#pre_render' => array('form_pre_render_fieldset', 'example_pre_render_bar'),
    '#theme_wrappers' => array('fieldset'),
  );

I.e. The FAPI definition wouldn't change, and the #pre_render functions are appended to the #pre_render array in the order they're defined (and perhaps have a #override_defaults property which can be applied when the defaults are not wanted).

However, I'm open to any of the options above - the most important aspect for me is that whatever approach ends up being chosen is well-documented.

The problem with #16 is that this makes it impossible to *override* a #type's #pre_render property. As stated in sun's OP, sometimes you want to add to the default properties and sometimes you want to override them. That's why cannot leave it all to the system.

I like both #9 and #10, the difference is not very important IMO.

subscribe

Yet another possibility that addresses delayed evaluation of #type (see #15):

// form code
$form['foo'] = array(
  '#type' => 'fieldset',
  '#pre_render' => array('example_pre_render_bar', 'element_type_pre_render'),
);

// in common.inc
function element_type_pre_render($element) {
  // element_info_property() in #10, but this adds a 3rd param for default if type doesn't have it
  foreach (element_info_property($element['#type'], '#pre_render', array()) as $function) {
    $element = $function($element);
  }
  return $element;
}

Thanks all! You raised some very good points and ideas so far.

Just throwing it out there: I wonder whether there'd be a way of making #10 a little more attractive and less verbose, such as:

$build = array(
  '#markup' => 'something',
  '#pre_render' => array('some_function') + element_info_property('markup', 'pre_render'),
  '#theme_wrappers' => element_info_property('markup', 'theme_wrappers') + array('some_wrapper');

@sun - I tried the + operator but it does not merge the arrays as desired.

Sure -- would need an additional trick, as in attached patch. Thoughts?

Cute, but the drupal_map_assoc() wouldn't be good for #attached, #attributes, and #upload_validators.

Although badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).

It is actually really nice to have all of the parameters to work with early on in the process, and the OO method would be great for Drupal 8.

I personally do not like adding necessary dependencies, so I would just create simple element classes that are added to an extended rendering class, such as a form for additional processing and rendering.

You could allow the constructor to have an override parameter that knocks out the element_info() properties during initialization;

<?php 

abstract class Element {
  protected $properties;
  protected $type;

  /**
   * The constructor allows the types primary properties to be overridden.
   */
  public function __construct($type, $properties = array()) {
    $this->type = $type;
    $this->properties = $properties + element_info($type);
  }

  public function type() {
    return $this->type;
  }
  
  /**
   * The main rendering function.
   */
  public function __toString() {
  }
}

class TextField extends Element {
  
  protected function __construct($properties = array()) {
    parent::__construct($type, $properties);
  }

  static function load($properties = array()) {
    return new TextField($properties);
  }
}

?>

If you want to add, etc, do it separately afterwards.

<?php

$elements['elm'] = TextField::load(array('title' => t('My title override'), 'element_validate' => array('my_override_validator')))
    ->appendPreRender('my_additional_pre_render');
?>

The OO way seems so much nicer and cleaner. (PS: I haven't programmed OO in PHP for nearly 4 years, so the code is rusty)

This is a D8 possible solution.

I've been wonder why there is a distinction between #validate and #element_validate in core, but if this could be merged, it is possible to create a namespace to "protect" the system defined element properties by defining #element_xxx like properties, such as #element_validate, #element_pre_process, etc. Then as the form is built, the properties with this prefix could be appended to the main elements properties.

This would allow programmers to override the defaults without any special magic by specifying the #element_xxx property, or just the #xxx property to add to the existing property.

This seems like a clean solution without the WTF, but the only hurdle is the current #validate / #element_validate pairing.

Very rough outline, by someone not that familiar with the D7 form processing:

function form_builder($form_id, $element, &$form_state) {
  // Initialize as unprocessed.
  $element['#processed'] = FALSE;

  // Use element defaults.
  if (isset($element['#type']) && empty($element['#defaults_loaded']) && ($info = element_info($element['#type']))) {
    // Overlay $info onto $element, retaining preexisting keys in $element.
    $element += $info;
    foreach ($element as $key => $value) {
      // Append any existing elemental properties onto the $element.
      if (strpos($key, '#element_') === 0 && is_array($value)) {
        $property_key = str_replace('#element_', '#', $key);
        if (!isset($element[$property_key])) {
          $element[$property_key] = array();
        }
        $element[$property_key] += $value;
      }
    }
    $element['#defaults_loaded'] = TRUE;
  }
  ....
}

/**
 * Implements hook_element_info().
 */
function system_element_info() {

  $types['fieldset'] = array(
    '#collapsible' => FALSE,
    '#collapsed' => FALSE,
    '#value' => NULL,
    '#element_process' => array('form_process_fieldset', 'ajax_process_form'),
    '#element_pre_render' => array('form_pre_render_fieldset'),
    '#theme_wrappers' => array('fieldset'),
  );

}

For D7 to avoid making an API change, simply add an extra condition to exclude the #element_validate property:

function form_builder($form_id, $element, &$form_state) {
  // Initialize as unprocessed.
  $element['#processed'] = FALSE;

  // Use element defaults.
  if (isset($element['#type']) && empty($element['#defaults_loaded']) && ($info = element_info($element['#type']))) {
    // Overlay $info onto $element, retaining preexisting keys in $element.
    $element += $info;
    foreach ($element as $key => $value) {
      // Merge in #element_xxx like properties onto the $element, with the exception
      // of #element_validate.
      if ($key != '#element_validate' && strpos($key, '#element_') === 0 && is_array($value)) {
        $property_key = str_replace('#element_', '#', $key);
        if (!isset($element[$property_key])) {
          $element[$property_key] = array();
        }
        $element[$property_key] += $value;
      }
    }
    $element['#defaults_loaded'] = TRUE;
  }
  ....
}

Re #26: I'm on the fence as to whether this is normal or major, but it is, for sure, a WTF. I've been bitten by it several times with #process when writing modules in D6, and it's only gonna get worse for D7 contrib authors, because of the greater usage of #pre_render and #theme_wrappers.

Re #28: Even that is an API change, because it means hook_form_alter() implementations that currently unset #process, #pre_render, etc., would need to unset #element_process, #element_pre_render, etc. as well. Something along those lines might be a good approach for D8, but I'm afraid not workable for D7.

As per #17, I don't think there's any way to fix the WTF entirely for D7. For sure, it's something we need to improve in D8.

But, as per #10, at the very least, we can provide a function to let contrib authors properly extend properties, when that's what they want. Here's just a simple fix of #10. It's not a fully transparent system, as #16 wanted, but I think it's the best we can do for D7.

Re #20: I'm not that concerned about array_merge() being difficult to use. I'm more concerned about people not realizing that when they set instance properties that they are overriding per-type properties. Changing array_merge() to + doesn't solve that, and brings in more problems (#23) than it solves.

+++ modules/field_ui/field_ui.admin.inc	6 Oct 2010 22:12:44 -0000
@@ -1647,13 +1647,7 @@ function field_ui_field_edit_form($form,
-  // @todo Fieldset element info needs to be merged in order to not skip the
-  //   default element definition for #pre_render. While the current default
-  //   value could simply be hard-coded, we'd possibly forget this location
-  //   when system_element_info() is updated. See also form_builder(). This
-  //   particular #pre_render, field_ui_field_edit_instance_pre_render(), might
-  //   as well be entirely needless though.
-  $form['instance'] = array_merge(element_info('fieldset'), array(
+  $form['instance'] = array(
     '#tree' => TRUE,
     '#type' => 'fieldset',
     '#title' => t('%type settings', array('%type' => $bundles[$entity_type][$bundle]['label'])),
@@ -1661,8 +1655,10 @@ function field_ui_field_edit_form($form,

@@ -1661,8 +1655,10 @@ function field_ui_field_edit_form($form,
       '%field' => $instance['label'],
       '%type' => $bundles[$entity_type][$bundle]['label'],
     )),
-    '#pre_render' => array('field_ui_field_edit_instance_pre_render'),
-  ));
+    // Ensure field_ui_field_edit_instance_pre_render() gets called in addition
+    // to, not instead of, the #pre_render function(s) needed by all fieldsets.
+    '#pre_render' => array_merge(array('field_ui_field_edit_instance_pre_render'), element_info_property('fieldset', '#pre_render', array())),
+  );

Looking at the minus signs, how does this even manage to work in HEAD? That array_merge() syntax is resulting in #pre_render overriding, not extending the type-defined #pre_render.

I agree that #30 is the way to go. Bot is green so RTBC.

#29: drupal.element_info_property.29.patch queued for re-testing.

#29: drupal.element_info_property.29.patch queued for re-testing.

Committed to CVS HEAD. Thanks.

Ha, unbelievable that this was our final solution. So the end resolution is: if you need to add your own #process or #pre_render (or any other property) to a form element, you have to manually merge it yourself.

In an example, if you have a "checkboxes" element and you want to add your own #pre_render function, instead of:

$form['my_checkboxes']['#pre_render'][] = 'my_pre_render';

You have to do:

$form['my_checkboxes']['#pre_render'] => array_merge(array('my_pre_render'), element_info_property('checkboxes', '#pre_render', array()));

Swell.

This should probably be opened as a new issue, this is ridiculous and will inevitably lead to developer confusion. This problem wasn't fixed, it was covered up. :P

I agree.

Right now there is a bug on conditional_fields module because it is adding a pre_render function to form elements without using element_info_property: #1215826: Dependent required checkboxes and radio lack a label when required

Subscribe.

So the end resolution is: if you need to add your own #process or #pre_render (or any other property) to a form element, you have to manually merge it yourself.

Would anyone be willing to confirm if this is still the reality for D7? My observations (and numerous hours debugging an issue with a #pre_render addition) seem to indicate the statement above still applies. However, it's now 4 years later and I'm not sure if other discussions have come up elsewhere around this.

If the above is still true, it seems that DX may greatly benefit from some API documentation updates, notably at https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h..., though that would be a separate issue.