Problem/Motivation

Panels (7.x-3.x-dev) throws error notices when choosing a region style. This can be reproduced by creating a panel/mini-panel, then choose a region style and save. There are about 20 error notices like:

Notice: Undefined property: stdClass::$type in panels_renderer_standard->prepare_panes() (line 237 of /bla/bla/sites/all/modules/contrib/panels/plugins/display_renderers/panels_renderer_standard.class.php).
Notice: Undefined property: stdClass::$subtype in panels_renderer_standard->prepare_panes() (line 237 of /bla/bla/sites/all/modules/contrib/panels/plugins/display_renderers/panels_renderer_standard.class.php).

I traced the issue that introduces the patch after which this issue starts: #1797298: Form caching cause references to break, preventing save of stylizer settings forms

The following lines in panels_renderer_editor.class.php are what start causing this:

    // Copy settings from form state back into the cache.
    if(!empty($form_state['values']['settings'])) {
      $this->cache->display->content[$pid]->style['settings'] = $form_state['values']['settings'];
    }

I think this was added to fix a problem in Panopoly (#1734772: Preview Widget on panels_edit_style_settings_form).

Proposed resolution

I think the issue #1797298: Form caching cause references to break, preventing save of stylizer settings forms is only relevant for panes, not regions. Therefore I suggest checking if the $type is actually a pane and not something else:

if(!empty($form_state['values']['settings']) && $type == 'pane') {

I'll attach a patch.

Remaining tasks

This needs to be verified, perhaps also with the Panopoly people.

#1797298: Form caching cause references to break, preventing save of stylizer settings forms
#1734772: Preview Widget on panels_edit_style_settings_form

CommentFileSizeAuthor
#7 undefined_property_notices_fix-2098515-7.patch939 bytesniko-
#3 undefined_property_notices_fix-2098515-2.patch727 bytesOnkelTem
#1 undefined_property_notices_fix-2098515-1.patch681 bytesdagomar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dagomar’s picture

dagomar CreditAttribution: dagomar commented
FileSize
undefined_property_notices_fix-2098515-1.patch681 bytes
OnkelTem’s picture

OnkelTem CreditAttribution: OnkelTem commented

I faced this problem too.

I too have reached the same code point but I was not sure how this should be fixed properly.

The way it is suggested by the patch provided, we skip setting settings cache for the region. However, I don't know do we need it really. So I've come up to a bit more timid patch. Attaching.

OnkelTem’s picture

OnkelTem CreditAttribution: OnkelTem commented
Issue summary: View changes
FileSize
undefined_property_notices_fix-2098515-2.patch727 bytes
hawkeye.twolf’s picture

hawkeye.twolf
they/them or he/him
Primary language English
Location ᏙᎩᏯᏍᏗ Unalatogiyasdi, Tsalaguwetiyi (Cherokee country)
CreditAttribution: hawkeye.twolf commented

Verified the issue in fresh D7.26, ctools 1.4, and Panels 3.4. Both patches worked for me, though I'm not sure which is the better approach.

*bump*bump* needs review by maintainer to choose the better patch.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Needs review » Reviewed & tested by the community

Ok, lets see what japerry says.

ergophobe’s picture

ergophobe CreditAttribution: ergophobe commented

I only tried patch #3 since it operates on the base class and is likely to catch more cases. In any case, it works fine.

niko-’s picture

niko- CreditAttribution: niko- commented
Status: Reviewed & tested by the community » Needs review
FileSize
undefined_property_notices_fix-2098515-7.patch939 bytes

Patches above not really fix this issue.
While I debug it I found that pane and region settings must be saved in different properties of display object.
For pane settings:

$this->cache->display->content[$pid]->style['settings']

For region settings:

$this->cache->display->panel_settings['style_settings'][$pid]

But original code saves pane and region settings in the location for panes settings (So warnings apeare).
Moreover this trouble leads to the fact that any custom settings for region will not be saved.
Also possibly other settings forms fall into this issue.
Attached patch fixes this issue such way as I described.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Reviewed & tested by the community

awesome catch!

  • Commit ed5fd8c on 7.x-3.x authored by niko-, committed by japerry: 
    Issue #2098515 by Niko-, dagomar, OnkelTem: Setting region style returns...
japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry commented
Status: Reviewed & tested by the community » Fixed

Yup, this tested out good for me, committed patch #7.

Status: Fixed » Closed (fixed)

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

Mugé’s picture

Mugé CreditAttribution: Mugé commented

Thank you!

maxplus’s picture

maxplus CreditAttribution: maxplus commented

Thanks!

notices disappeared after using the current dev version (including this patch)

JeroenKool’s picture

JeroenKool CreditAttribution: JeroenKool commented

Thanks!

ergophobe’s picture

ergophobe CreditAttribution: ergophobe commented

I just tried the current dev which includes patch #7 and I'm still getting these errors.

Patch #3 fixed it for me, but #7 doesn't.

I'm running CTools 1.6 RC1 and the current dev version of Panlizer and Panels

ergophobe’s picture

ergophobe CreditAttribution: ergophobe commented
Status: Closed (fixed) » Needs work

Sorry for the noise - my bad

ergophobe’s picture

ergophobe CreditAttribution: ergophobe commented
Status: Needs work » Closed (fixed)