Description of the need

An interesting issue popped up when porting the Comment Alter module. When adapting the comment_alter_form_field_ui_field_edit_form_alter(&$form, &$form_state, $form_id) function (see https://git.drupalcode.org/project/comment_alter/-/blob/7.x-1.x/comment_...) to Backdrop, I've traced why it was not saving new values for custom fields on a content type edit page such as, for example, admin/structure/types/manage/ticket/fields/field_status to the following difference between Drupal 7 and Backdrop cores.

Here is how the original function looks like on https://git.drupalcode.org/project/drupal/-/blob/7.x/modules/field/field...

/**
 * Stores an instance record in the field configuration database.
 *
 * @param $instance
 *   An instance structure.
 * @param $update
 *   Whether this is a new or existing instance.
 */
function _field_write_instance($instance, $update = FALSE) {
  $field = field_read_field($instance['field_name']);
  $field_type = field_info_field_types($field['type']);

  // Set defaults.
  $instance += array(
    'settings' => array(),
    'display' => array(),
    'widget' => array(),
    'required' => FALSE,
    'label' => $instance['field_name'],
    'description' => '',
    'deleted' => 0,
  );

  // Set default instance settings.
  $instance['settings'] += field_info_instance_settings($field['type']);

  // Set default widget and settings.
  $instance['widget'] += array(
    // TODO: what if no 'default_widget' specified ?
    'type' => $field_type['default_widget'],
    'settings' => array(),
  );
  // If no weight specified, make sure the field sinks at the bottom.
  if (!isset($instance['widget']['weight'])) {
    $max_weight = field_info_max_weight($instance['entity_type'], $instance['bundle'], 'form');
    $instance['widget']['weight'] = isset($max_weight) ? $max_weight + 1 : 0;
  }
  // Check widget module.
  $widget_type = field_info_widget_types($instance['widget']['type']);
  $instance['widget']['module'] = $widget_type['module'];
  $instance['widget']['settings'] += field_info_widget_settings($instance['widget']['type']);

  // Make sure there are at least display settings for the 'default' view mode,
  // and fill in defaults for each view mode specified in the definition.
  $instance['display'] += array(
    'default' => array(),
  );
  foreach ($instance['display'] as $view_mode => $display) {
    $display += array(
      'label' => 'above',
      'type' => isset($field_type['default_formatter']) ? $field_type['default_formatter'] : 'hidden',
      'settings' => array(),
    );
    if ($display['type'] != 'hidden') {
      $formatter_type = field_info_formatter_types($display['type']);
      $display['module'] = $formatter_type['module'];
      $display['settings'] += field_info_formatter_settings($display['type']);
    }
    // If no weight specified, make sure the field sinks at the bottom.
    if (!isset($display['weight'])) {
      $max_weight = field_info_max_weight($instance['entity_type'], $instance['bundle'], $view_mode);
      $display['weight'] = isset($max_weight) ? $max_weight + 1 : 0;
    }
    $instance['display'][$view_mode] = $display;
  }

  // The serialized 'data' column contains everything from $instance that does
  // not have its own column and is not automatically populated when the
  // instance is read.
  $data = $instance;
  unset($data['id'], $data['field_id'], $data['field_name'], $data['entity_type'], $data['bundle'], $data['deleted']);

  $record = array(
    'field_id' => $instance['field_id'],
    'field_name' => $instance['field_name'],
    'entity_type' => $instance['entity_type'],
    'bundle' => $instance['bundle'],
    'data' => $data,
    'deleted' => $instance['deleted'],
  );
  // We need to tell drupal_update_record() the primary keys to trigger an
  // update.
  if ($update) {
    $record['id'] = $instance['id'];
    $primary_key = array('id');
  }
  else {
    $primary_key = array();
  }
  drupal_write_record('field_config_instance', $record, $primary_key);
}

Pay special attention to the comment and the line following it:

// The serialized 'data' column contains everything from $instance that does
  // not have its own column and is not automatically populated when the
  // instance is read.
  $data = $instance;

Now see how the same function looks on https://github.com/backdrop/backdrop/blob/7cc9968d3c1f8885a9d126668b1a8c...

/**
 * Stores an instance record in the field configuration database.
 *
 * @param $instance
 *   An instance structure.
 * @param $update
 *   Whether this is a new or existing instance.
 */
function _field_write_instance($instance, $update = FALSE) {
  $field = field_read_field($instance['field_name']);
  $field_type = field_info_field_types($field['type']);

  // Set default widget and settings.
  $instance['widget'] += array(
    // TODO: what if no 'default_widget' specified ?
    'type' => $field_type['default_widget'],
    'settings' => array(),
  );
  // If no weight specified, make sure the field sinks at the bottom.
  if (!isset($instance['widget']['weight'])) {
    $max_weight = field_info_max_weight($instance['entity_type'], $instance['bundle'], 'form');
    $instance['widget']['weight'] = isset($max_weight) ? $max_weight + 1 : 0;
  }
  // Check widget module.
  $widget_type = field_info_widget_types($instance['widget']['type']);
  $instance['widget']['module'] = isset($widget_type['module']) ? $widget_type['module'] : '';
  $instance['widget']['settings'] += field_info_widget_settings($instance['widget']['type']);

  // Make sure there are at least display settings for the 'default' display
  // mode, and fill in defaults for each display mode specified in the
  // definition.
  $instance['display'] += array(
    'default' => array(),
  );
  foreach ($instance['display'] as $view_mode => $display) {
    $display += array(
      'label' => 'hidden',
      'type' => isset($field_type['default_formatter']) ? $field_type['default_formatter'] : 'hidden',
      'settings' => array(),
    );
    if ($display['type'] != 'hidden') {
      $formatter_type = field_info_formatter_types($display['type']);
      $display['module'] = isset($formatter_type['module']) ? $formatter_type['module'] : '';
      $display['settings'] += field_info_formatter_settings($display['type']);
    }
    // If no weight specified, make sure the field sinks at the bottom.
    if (!isset($display['weight'])) {
      $max_weight = field_info_max_weight($instance['entity_type'], $instance['bundle'], $view_mode);
      $display['weight'] = isset($max_weight) ? $max_weight + 1 : 0;
    }
    $instance['display'][$view_mode] = $display;
  }

  // Include only defined data in the configuration file.
  $instance_data = array_intersect_key($instance, field_defaults_instance());

  $config = config('field.instance.' . $instance['entity_type'] . '.' . $instance['bundle'] . '.' . $instance['field_name']);
  $config->setData($instance_data);
  $config->save();
}

As you see the very end of the Backdrop version of the function is quite restrictive, in fact it's just the opposite of inclusive Drupal 7 version:

// Include only defined data in the configuration file.
  $instance_data = array_intersect_key($instance, field_defaults_instance());

Git blaming took me to https://github.com/backdrop/backdrop/commit/3ab41c27beaf214a2902803a9fb2... showing the change was done as part of https://github.com/backdrop/backdrop/pull/178

Basically, with this change the value of custom fields stored within the $instance variable never gets into the account, so not processed further.

As soon as I pass the value or $instance directly to $instance_data the ported module starts working as expected.

Proposed solution

I'm not sure exactly why it needed to:

Include only defined data in the configuration file.

whereas Drupal 7 is doing just the opposite:

// The serialized 'data' column contains everything from $instance that does // not have its own column and is not automatically populated when the // instance is read.

but I tested many times passing the data with and without array_intersect_key function and had to conclude the inclusive method is not hurting anything while at the same time making it possible to easily port some Drupal 7 modules like comment_alter relying on the same core functions as Drupal 7 did.

Of course, it's always possible to come up with custom solution completely bypassing the core functions, but before digging into alternative ways, I thought to reach out to Backdrop core maintainers/committers if they would consider bringing back the inclusive character of respective code lines in Drupal 7 version.

GitHub Issue #: 
5875