ctools_fields_get_field_formatter_settings_form() not fully compatible to field_ui [#2152405]

Problem/Motivation

If a field formatter has cascaded settings e.g. $settings['markup'] = array('wrapper' => 'div', 'classes' => 'i-m-a-class') it's not possible to set the settings in panels / panelizer.
As far as I see this is because ctools_fields_get_field_formatter_settings_form() doesn't set the #tree property to the field formatter settings form.
But field_ui.admin.inc (line 912) does this:

  $table = array(
    '#type' => 'field_ui_table',
    '#tree' => TRUE,

...

      if ($settings_form) {
        $table[$name]['format']['#cell_attributes'] = array('colspan' => 3);
        $table[$name]['format']['settings_edit_form']

This means the settings form is also affected by the #tree property.
Debugging further shows that without the #tree property the function ctools_entity_field_content_type_formatter_styles_submit() can't find the cascaded field formatter settings in the form values and thus those settings are ignored.

Proposed resolution

Set the #tree property in ctools_fields_get_field_formatter_settings_form().

Remaining tasks

Reviews needed.

User interface changes

None.

API changes

The structure of the form values changes.

Comment	File	Size	Author
	ctools-casceded-field-formatter-settings-not-stored.patch	641 bytes	das-peter

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

das-peter CreditAttribution: das-peter commented 8 December 2013 at 15:26

Title:

ctools_fields_get_field_formatter_settings_form() not fully compatibly to field_ui

» ctools_fields_get_field_formatter_settings_form() not fully compatible to field_ui

Comment #2

das-peter CreditAttribution: das-peter commented 8 December 2013 at 18:48

Status:

Active

» Needs review

Comment #3

kristiaanvandeneynde

Dutch

Antwerp, Belgium

CreditAttribution: kristiaanvandeneynde commented 20 March 2014 at 10:43

Status:

Needs review

» Reviewed & tested by the community

Tested to work, thanks for the patch!

Comment #4

Anybody

German

Porta Westfalica

CreditAttribution: Anybody commented 18 July 2014 at 14:30

Thanks a lot. It would be great to have this in the next .dev release. Is that possible?

Comment #5

Anybody

German

Porta Westfalica

CreditAttribution: Anybody commented 11 August 2014 at 16:09

Priority:

Normal

» Major

Who will have a look at this and take this into dev? :)
This is really problematic for some modules. I hope it is okay though to set the priority up.

CreditAttribution: japerry commented 19 December 2014 at 00:30

Status:

Reviewed & tested by the community

» Fixed

Looks good, Committed.

Comment #10

19 December 2014 at 00:31

japerry committed 0117b96 on 7.x-1.x authored by das-peter
```
Issue #2152405 by das-peter:...
```

Comment #11

kristiaanvandeneynde

Dutch

Antwerp, Belgium

CreditAttribution: kristiaanvandeneynde commented 19 December 2014 at 10:00

Ugh, just noticed something after this got committed:

+++ b/includes/fields.inc
@@ -78,6 +78,7 @@ function ctools_fields_get_field_formatter_settings_form($field, $formatter_type
     $settings_form = $function($field, $instance, $view_mode, $form, $form_state);
+    $settings_form['#tree'] = TRUE;
     if ($settings_form) {

Should be:

+++ b/includes/fields.inc
@@ -78,6 +78,7 @@ function ctools_fields_get_field_formatter_settings_form($field, $formatter_type
     $settings_form = $function($field, $instance, $view_mode, $form, $form_state);
     if ($settings_form) {
+      $settings_form['#tree'] = TRUE;

Otherwise the if-statement is always TRUE.

Comment #12

maximpodorov CreditAttribution: maximpodorov commented 23 December 2014 at 11:25

I created the new issue and attached the patch (using the change kristiaanvandeneynde proposed) here: #2397709: Set #tree property in the proper place