Is there any reason why Panels returns NULL?
As far I can see in API and index.php:
http://api.drupal.org/api/function/menu_execute_active_handler/6
http://drupalcode.org/viewvc/drupal/drupal/index.php?revision=1.94&view=...
It should return one of valid values: MENU_SITE_OFFLINE, MENU_ACCESS_DENIED or MENU_NOT_FOUND.
This issue is raised because of incompatibility with WSOD module (from Diagnostic Tool) #717416: Incompatibility with Panels/Page Manager module when panels have "Disable Drupal regions" selected
And I was wondering if it's expected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Status: Active » Closed (works as designed)

Yes. The only way to set the show_blocks flag to theme('page') is to call it manually. There is no way to tell the menu system to add that flag to theme('page').

It is a perfectly valid workflow to call print theme('page', $show_blocks) and then return NULL to indicate that you have rendered the data yourself. WSOD module is making an error an assuming that a NULL is a crash.

kenorb’s picture

Status: Closed (works as designed) » Active

Is there any way to define its own constant?
Current:

/**
 * @name Menu status codes
 * @{
 * Status codes for menu callbacks.
 */

define('MENU_FOUND', 1);
define('MENU_NOT_FOUND', 2);
define('MENU_ACCESS_DENIED', 3);
define('MENU_SITE_OFFLINE', 4);

When you do in ./panels_node/panels_node.module

define('MENU_IGNORE_THEME', 0); // or any other number

/**
 * Pass through to the panels content editor.
 */
function panels_node_edit_content($node) {
...
  print theme('page', panels_edit($display, "node/$node->nid/panel_content", $content_types), FALSE);
}

Or just return this:

  return 0;

Most of the code which has broken menu callback, return just NULL and it's confusing.

Note: If you return 0, isset() will still return false, and page will not be rendered.

elseif (isset($return)) {

See: http://php.net/manual/en/function.isset.php

kenorb’s picture

Category: feature » support
Status: Needs review » Active
kenorb’s picture

Category: support » feature
Status: Active » Needs review
FileSize
691 bytes

Please check the following patch.
For you it doesn't change anything, for me yes.
Because there is a difference between NULL and 0
Thank you.

kenorb’s picture

Category: support » feature
Status: Active » Needs review
FileSize
652 bytes

Patch as I can see in API way:
http://drupal.org/node/224333#theme_page

function mymodule_page_callback() {
  // Assemble page output here...

  // Return page output for Drupal.
  return theme('page', $output);
}

It's the correct way to pass already rendered page with custom arguments?

kenorb’s picture

Category: feature » bug
Priority: Normal » Minor
kenorb’s picture

FileSize
891 bytes

Ignore patch #3, check this.
Sorry, I couldn't test it.

kenorb’s picture

If you need to alter page attributes, you can use the following approach. This can be used to alter the page output for one of your module's page callbacks. For example, to have one of your module's page callbacks not display any sidebars:

function mymodule_page_callback() {
  // Assemble page output here...

  // Return page output for Drupal, hide blocks.
  return theme('page', $output, FALSE);
}

Source: http://drupal.org/node/224333#theme_page

Isn't exactly what we trying to achieve here?
"Return page output for Drupal, hide blocks"?

merlinofchaos’s picture

When you do that, don't you get the page theming twice? Certainly you used to. I don't remember hearing about any changes to that.

kenorb’s picture

I'm not sure, I've to test it as well.
http://drupal.org/node/428744#comment-1617886

kenorb’s picture

Tested in index.php:

    $return = menu_execute_active_handler();
+ $return = theme('page', 'blah blah', FALSE, FALSE);

It does duplicate.

This doesn't:

  $return = menu_execute_active_handler();
+  print theme('page', 'dupa jasia', FALSE, FALSE);
+  $return = 0;

So the patch #4 will work.

merlinofchaos’s picture

I'm ok with patch in #4, I think, but it needs a comment to explain why it's returning 0.

EvanDonovan’s picture

@kenorb: As per merlin's request, can you add the comment?

It could say something like:

"Return 0 rather than NULL so modules that are invoked later in the page request can know that page content rendered."

kenorb’s picture

Return 0, because if there is assignment in index.php

$return = menu_execute_active_handler();

code expecting that function (menu handler) will return anything.
Zero, because we don't want to execute none of the block within index.php with following conditions: is_int($return), isset($return).
Zero, rather than NULL, because module that are invoked later know that page content has been rendered correctly.

merlinofchaos’s picture

I mean...comment in the code.

kenorb’s picture

Ah, sorry.
Commented in the patch.

EvanDonovan’s picture

I think this is good. Might possibly want to also indicate in the code comment that returning 0 will still bypass block rendering.

Letharion’s picture

Component: Code » Panel nodes
Status: Needs review » Closed (won't fix)

This patch is both both really old by now, and for "Panel Nodes", a module that will be deprecated in favor of Panelizer shortly. #1353542: Upgrade path from Panel Nodes to Panelizer. Because of it's ago, and soon to be irrelevance, I'm closing this issue.

EvanDonovan’s picture

Status: Closed (won't fix) » Needs work

I don't think the patch should exclusively be for Panel Nodes; as far as I know, the issue happens on regular panels pages as well, and still causes an endless loop if Diagnostic Tools is enabled. Thus, I think it is still needing a more comprehensive fix.

Letharion’s picture

Assigned: Unassigned » merlinofchaos
Status: Needs work » Needs review
DamienMcKenna’s picture

vijaycs85’s picture

Status: Needs review » Postponed (maintainer needs more info)
Issue tags: +Needs issue summary update

We might need to update the issue summary with some details, perhaps steps to reproduce the issue.

DamienMcKenna’s picture

Bumping to the next release, but I suspect it may never be fixed.

japerry’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

Closing this issue as outdated as Drupal 6 ctools is not supported. If this issue is relevant for Drupal 7, feel free to re-open and mark for Drupal 7 (or 8)