Problem/Motivation

When using entity_field_value plugin to limit visibility of a pane on a taxonomy field with Hierarchical select widget, we found an issue that is causing visibility rule settings not to be set and saved.

This is truly widget dependent, switching back to select list widget on this taxonomy field helped to resolve the issue, but it's not an option as we need to use hierarchical select widget on node edit page.

It looks like the problem is in this snippet:

    $form_state = array(
      'display' => &$this->display,
      'pane' => &$pane,
      'ajax' => TRUE,
      'title' => t('Configure visibility rule for !subtype_title', array('!subtype_title' => $subtype['title'])),
      'test' => &$pane->access['plugins'][$id],
      'plugin' => ctools_get_access_plugin($pane->access['plugins'][$id]['name']),
      'url' => url($url, array('absolute' => TRUE)),
    );

    $output = ctools_modal_form_wrapper('panels_edit_configure_access_test_form', $form_state);

While $form_state['display'] should have a reference to $this->display and modifying $form_state['display'] should be propagated to $this->display (which is used for saving display settings later, this does not happen sometimes.
The reason for this is $form_state is passed by reference, but under certain circumstances it is not the same object when it is returned into this scope. This happens primarily on form_get_cache, when $form_state is being rebuilt when form used on visibility settings widget uses AJAX and triggers rebuild.

Proposed resolution

I propose to make sure we assign back the $form_state['display'] to $this->display as this is critical to keep settings saved.

Remaining tasks

Provide a patch

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#3 panels-broken_visibility_settings-ajax-2605094-3.patch820 bytesasgorobets
#2 panels-broken_visibility_settings-ajax-2605094-1.patch776 bytesasgorobets
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

asgorobets created an issue. See original summary.

asgorobets’s picture

asgorobets CreditAttribution: asgorobets at FFW commented
Status: Active » Needs review
FileSize
panels-broken_visibility_settings-ajax-2605094-1.patch776 bytes

Adding a patch

asgorobets’s picture

asgorobets CreditAttribution: asgorobets at FFW commented
FileSize
panels-broken_visibility_settings-ajax-2605094-3.patch820 bytes

ouch, patch in #2 was applied to a different form, and it was also incomplete.

Attaching a corrected patch, that applies to 'panels_edit_configure_access_test_form' rather than 'panels_edit_configure_access_settings_form'.
Also had to restore $pane variable reference as well, as Delete action was not working properly either when form is retrieved from cache.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and at Propeople (now part of FFW), FFW for FFW commented
Status: Needs review » Reviewed & tested by the community
japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented
Status: Reviewed & tested by the community » Needs review

I don't believe this is actually RTBC'd yet

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and at FFW for FFW commented

Of course you don't, @jappery. But the same problems exists at whole panels_renderer_editor object.

BR0kEN’s picture

BR0kEN
Primary language Ukrainian
CreditAttribution: BR0kEN as a volunteer and at FFW for FFW commented

No need to go far away, let's look at the ajax_style_settings method.

This code from a form submit callback from a custom module (pay attention on @todo):

  // Save display style settings.
  // @see \panels_renderer_editor::get_style()
  // This defaults will be applied when new region will be created.
  // @see ctools_api_panels_flexible_add_item_form_submit()
  if (isset($form_state['type'], $form_state['renderer']) && 'display' === $form_state['type']) {
    // Configuration MUST be stored in cache, and, when "Update and save" button
    // will be clicked, then all data from cache will transferred to database.
    $renderer =& $form_state['renderer'];
    $renderer->cache->display->panel_settings['style_settings']['default'] = $form_state['conf'];
    // @todo Create a patch to Panels and remove this line.
    panels_edit_cache_set($renderer->cache);
  }

This can be easily fixed by adding $this->cache = $form_state['renderer']->cache; before if (isset($this->cache->style)) { at the ajax_style_settings method.

Need to debug to know why this happens. Also, constructions like this 'renderer' => &$this,, do not have any sense, because all objects in PHP are passed by reference.