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.
Recent comments
@indigoxela - I agree, I mentioned the SVG topic specifically for the dev meeting. I think we'll definitely want Nate involved in that discussion. Good call!
Apr 18th Weekly Meetings
Hi colbyat, re display in the browser: the problem is that when a 500 occurs, this PHP process died before being able to render anything. So that won't work. Logging is possible, but...
Displaying 500 errors
SVG's are a great topic for the design meeting, but also for the dev meeting. Two currently active issues related to SVG overlap a bit when it comes to security aspects. https://...
Apr 18th Weekly Meetings