Problem/Motivation

panels_renderer_editor->ajax_style_type() calls ctools_content_get_subtype() and provides a pane id. But FPP and og_fpp (correctly) filter out all non-reusable panes in their content_types callback. This results in undefined index errors for non-reusable FPPs when editing their styles.

This is a Panels issue, primarily because of an architectural issue with Panels. It seems that the content_types callback is intended to filter for allowed types. See panels_common_get_allowed_types(), which eventually calls the content_types callback. The issue is that ctools_content_get_subtype() gets called later by panels_renderer_editor->ajax_style_type(), passing the pane id for a non-reusable pane and an undefined index is returned.

Proposed resolution

Decouple ajax_style_type() from calling ctools_content_get_subtype() and load the pane title by calling ctools_content_admin_title().

Remaining tasks

  • Reviews

User interface changes

n/a

API changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn’s picture

Figured out what needed to be called to load a title. Let's see how this fares with testing.

heddn’s picture

Status: Active » Needs review
mglaman’s picture

Fixes issue for me with Fieldable Panel Panes and styling (no more Pane style for ""). I wanted to RTBC, but I noticed the call for ctools_content_get_subtype() was no longer needed. Including patch to remove this line.

heddn’s picture

I'm OK with marking RTBC on #3 if you are @mglaman; the change to remove a non-used variable is a good one.

mglaman’s picture

Yep!

heddn’s picture

Status: Needs review » Reviewed & tested by the community
mrjmd’s picture

japerry’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +SprintWeekend2015

Looks good. Committed.

  • japerry committed 0341491 on 7.x-3.x authored by heddn
    Issue #2366389 by mglaman, heddn: Undefined index: title in...
das-peter’s picture

Status: Fixed » Needs review
FileSize
833 bytes

Sorry to re-open this but it seems like this change triggers a bug in ctools ctools_content_admin_title() / ctools_context_required::filter().
ctools_content_admin_title() says context is optional but doesn't handle it properly.
So NULL is passed on as context which will finally fail in ctools_context_required::filter() where the NULL is handled as object. This leads to an error like:
Fatal error: Call to a member function is_type() on a non-object in /Users/matthias-diez/dev/projects/postauto/sites/all/modules/contrib/ctools/includes/context.inc on line 147

I suggest we circumvent that now by just passing the display context like in other places. But we should fix the empty property handling in ctools_content_admin_title() eventually and ensure only a proper context / context array is passed on.

  • japerry committed 446e613 on 7.x-3.x authored by das-peter
    Issue #2366389 by das-peter: Send context to ctools_admin_title to...
japerry’s picture

Status: Needs review » Fixed

Hrmm yah I don't want to fix the admin title function quite yet, that'll take a bit more time to debug. It looks like there are a few functions there that haven't been really looked at since 2010, so I worry that moving even simple things like NULLs and FALSE (since some are strict comparisons) could cause problems.

In the meantime, this is committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.