It would be useful for themers to retain the original $content variable in template_preprocess_panels_pane().

Currently it overwrites $vars['content'] with $vars['content']->content and you lose everything else that was in $vars['content'] that isn't $vars['content']->content.

Since many themers probably rely on the $vars array being as it currently is we can't change that without potentially breaking a bunch of websites so I propose just adding another variable to $vars that includes all of $vars['content'] except for $vars['content']->content (personally I don't see the point in including $vars['content']->content twice and wasting resources).

Maybe it could go in $vars['content_metadata'] or $vars['content_definition'] or $vars['content_context'] or something.

Original bug report

The problem:

When you override template_preprocess_panels_pane() in your own theme, weirdness ensues when you just copy/paste and start altering anything which uses the $content variable within the provided function body. i.e. adding a another variation to the theme_hook_suggestions array.

How to reproduce

1. Install Drupal and Panels
2. Create a new theme
3. Override template_preprocess_panels_pane() in your own template.php
4. Verbatim copy/paste the function body from panels.module
5. Add a variation to the theme_hook_suggestions array: i.e. $vars['theme_hook_suggestions'][] = $base . $delimiter . $content->type . -'test';
6. Create new template called panels-pane--custom-test.tpl.php in your theme and add some test html (or the contents from panels-pane.tpl.php)
7. Create a new custom page and add a custom content panel
8. Clear your caches
9. Notice how the region in your custom page will appear empty.

Expected behaviour: you should see the output piped through the panels-pane--custom-test.tpl.php template.

How come?

The body of the template_preprocess_panels_pane() starts and ends with:

$content = &$vars['content'];
...
$vars['content'] = !empty($content->content) ? $content->content : ''; (line 1298 in panels.module)

The original $content variable within the panels.module context is an object with a few properties like 'type', 'subtype', 'title' and 'content'. At the end of the execution of the original function, the $vars['content'] gets overwritten with the 'content' property.

Overriding in template.php doesn't mean that the original template_preprocess_* won't be executed. It will be executed followed by the execution of the specific function in template.php passing $vars by reference to allow final tweaking. Since $vars['content'] is overwritten when the theme() function enters template.php, we only get a "lousy string" instead of a nice object.

Possible solution

Remove the assign at line 1298 fixes the issue and passes $vars['content'] as a nice object to possible preprocess overrides instead of a non-flexible string.

Files: 
CommentFileSizeAuthor
#11 panels-fix-preprocess_panels_pane-707484-6.patch461 bytesK3vin_nl
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#4 panels-preprocess_pane_preserve_content-1853282-4.patch711 bytesrooby
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

merlinofchaos’s picture

Changing the $content variable mid-stream would probably change people's templates, and that would lead to problems. So I don't think we can actually do that.

We can possibly rename the original $content variable so that it is preserved.

You *can* override the original template preprocess. In hook_theme, you can set this:

'override preprocess functions' => TRUE. Your theme can actually implement hook_theme() and set that for that and then NO preprocess functions except the theme's get called. (You can, then, if you like, call back to whichever ones you wish to use).

I'm not sure that Panels is likely to change anything here.

rooby’s picture

Title:template_preprocess_panels_pane() cannot be overridden properly» Preserve original $content variable in tempalte_preprocess_panels_pane()
Version:7.x-3.3» 7.x-3.x-dev
Category:Bug report» Feature request
Issue summary:View changes

I think it would be good to somehow preserve the original content variable so you have easy access to things like module, type, subtype, which are very likely useful to themers.

Another thing worth noting though based on the original post is that I recommend not doing this (steps 3 & 4):

When you override template_preprocess_panels_pane() in your own theme, weirdness ensues when you just copy/paste and start altering anything which uses the $content variable within the provided function body. i.e. adding a another variation to the theme_hook_suggestions array.

When you override a theme function you generally copy and paste the original function and then modify it to your liking.
When you are using template preprocess functions though this is not a good idea.
With a theme function override your function is replacing the original, however with a template preprocessor your function is running after the original (and potentially there are also others in addition to yours and the original) so you are modifying the existing variables array, not writing it from scratch.
It is not uncommon that if you copy and paste a preprocess function like that you will run into problems.

I recommend starting with an empty preprocess function, then if you do need to re-run parts of code from the original template_preprocess function just take those parts as required (they may need slight changes to account for the fact that they have already run once).

rooby’s picture

Status:Active» Needs review
StatusFileSize
new705 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Here is a patch that does it, with a few notes:

  1. I took the liberty of naming the variable, but someone who knows the panels/ctools plugin system better may have a better suggestion for that.
  2. I removed the duplicate content but if it is decided it is better to leave it I can make that change.
  3. I wasn't sure if it was possible for $content->content to be initially unset so I put a check anyway. The check may not be necessary.
rooby’s picture

StatusFileSize
new711 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Whoops.
Same patch except using clone to copy the object.

K3vin_nl’s picture

StatusFileSize
new595 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fix_preprocess_panels_pane.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

At first I could not understand why I could not have my custom panel style pass through both template_preprocess_panels_pane and my own preprocess function.
It turned out that the template_preprocess_panels_pane altered the data structure as described in this issue, causing my preprocess function to fail if it had run after template_preprocess_panels_pane.

Rooby, I tried your patch. While your solution would have allowed me to work around it, I don't think it is the proper way to fix this.

I think the solution might be much easier: keep the data structure in template_preprocess_panels_pane in tact and move the collapsing of $vars['content'] to a process function. That way you can have as many preprocess functions running in sequence as you want.

I've attached a patch as a proof of concept. A few quick checks seem to confirm this works as expected.

Status:Needs review» Needs work

The last submitted patch, 6: fix_preprocess_panels_pane.patch, failed testing.

K3vin_nl’s picture

StatusFileSize
new428 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Hopefully fixed the patch.

K3vin_nl’s picture

Status:Needs work» Needs review
K3vin_nl’s picture

K3vin_nl’s picture

StatusFileSize
new461 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Updated patch: Missed a line of code in the process function.

rooby’s picture

A possible downside of this approach is that it could break existing preprocess functions that people may have written that rely on the variables as they currently are.
So this solution would at least have to come with a notice for people to check their custom code.

This solution also still suffers in some ways from the same problem it is trying to solve because it is still overwriting the original $vars['content'].

With this patch if someone wants to use the original content in hook_process_panels_pane() or the template file then they will still have the problem.

rooby’s picture

Title:Preserve original $content variable in tempalte_preprocess_panels_pane()» Preserve original $content variable in template_preprocess_panels_pane()
K3vin_nl’s picture

Rooby, you are right, that might be an issue.

However since in the current situation it is pretty useless to have another preprocess function running after the original template_preprocess_panels_pane function I think the real world impact might be small.
For people who completely override the template_preprocess_panels_pane function it also shouldn't be an issue.

rooby’s picture

It doesn't really matter how useless it might be, people still could be doing it and panels is used by a very large amount of people and is extended by a number of other modules, so we need to exercise caution.

There is also still the matter of the problem not being fixed for the use of hook_process_panels_pane() and the use of the original content in the template file.

With this patch it would be possible for a person to add their own code in hook_preprocess_panels_pane() to get the original content for use later on but it would be better if the panels module did this so it was consistent for everyone.

In the end I see multiple potential issues with that approach.

The only possible issue with the other approach is that it will use a little more memory.
I don't think that it would be enough to cause concern but it could be benchmarked if anyone was worried.