Problem with Page.inc

I see the same thing. All of my top-level panel pages have previously added/shown an "Edit Panel" local menu task/tab. It has always appeared next to a simple "View" tab, much like the typical "View" and "Edit" tab for a node, except in this case it is for a panels page. I assume that "View" tab I've always seen was being added by the code in question, surrounded by the "if" check on line 317.

I put some debug information within the "if" statement to see what was contained in $page->menu at that point. Here is a print_r() of my $page->menu that previously used to execute the code within the "if" statement, but now no longer passes the test:

This obviously means that $page->menu['type]' is set, so that part of the "if" statement doesn't pass, and the $page->menu['type']=="normal" and therefore the later part of the test also now fails (since the type isn't 'tab' or 'default tab').

The comment above line 317, included in the issue summary, clearly indicates this code should exist to add a "View" tab so that "Edit" tabs can be added, and that is what previously worked with panels. Now, due to this change, the "View" local task does not get added and therefore prevents the "Edit Panel" tab/local task from being added.

Was there any issue or test case which prompted the original change? The change appears to completely contradict the entire purpose for those lines of code. Perhaps panels should be doing something entirely different, but perhaps there was also a different/better solution in the first place. Without more data about the reason for the original change, it feels like this one commit should be rolled back.

For what it's worth, I did a backtrace to see how this code was called. The stack was short, going from menu_execute_active_handler() in index.php to call_user_func_array() in menu.inc with page_manager_page_execute being the function to call. This is presumably because my panel page has directly specified a menu item (the normal "Dashboard" shown above). Although I haven't created a simple test case to reproduce this, I suspect creating any top-level panel page and giving it a menu item would reproduce this situation where "View" and "Edit Panel" no longer appear with 6.x-1.9.

Thanks for looking at this so quickly.

First, I need to amend my original description. I copied in the line of code from the wrong file, and my description was based on it, so it is backwards. The current page.inc (for 6.x-1.9) has the following line:

if (user_access('administer page manager') && (!isset($page->menu['type']) || in_array($page->menu['type'], array('tab', 'default tab'))))

This line does not have the not (!) operator before the in_array. The 1.8 verions did have the not operator, and it is the version that works for me. So in my description, instead of saying that the tabs returned when I removed the "!", I should have said that they returned when I added the "!" back in.

I'm not exactly sure what else you need, but I can tell you that

a) I have admin premissions on the page manager, so user_access('administer page manager') is true
b) $page->menu['type'] has the value of 'normal', so the !isset($page->menu['type']) is false
c) clearly, 'normal' is not in the array ('tab', 'default tab'), so !in_array($page->menu['type'], array('tab', 'default tab')) is true
d) also for the same reason, in_array($page->menu['type'], array('tab', 'default tab')) would be false, so both of the clauses that are or'ed together would be false, meaning that the block that builds the "fake" tab for 'View' would not execute.

I have uploaded a before and after screen shot from my web site.

The commit did fix a large number of cases. I'm not sure why this one isn't included there but I remember there being a lot of moving parts between this code and the menu system. I have a panel setup with a normal item and it isn't getting tabs but changing the negation isn't changing it. I'll continue looking but if it was possible to provide a simple test case I could install in a stock site it would help fix it faster.

One last comment. When I upgrade to 6.x-1.10 (which is now the most current version of ctools), I am seeing the same problem.

Hi YesCT

Thanks for your comments.

To update the ctools instance, I did not run drush, instead I downloaded it and ran update.php.

I tried your suggestion of creating the page in 6.10. When I do this, I do not see the menu items until I go into the Page.inc module and add the not operator back into line 317.

@wklyons Also, take your clean 6.10 you have with git, make a new branch, make the change in the one line to add the !, and do a git diff to 6.10 to create a patch. Uploading that patch here will make it easier for people to test adding that ! back in. http://drupal.org/node/1424598 is a good place to start. That is the documentation on the "Contributor task: Create a patch for a Drupal Core issue". You have to follow several links through a few pages to get to the git instructions to make the patch, but the documentation is pretty good.

Hi James, thanks for your response.

This sound reasonable to me, and should work fine for all. I will make this change for my client site, and assume that this will be included in a subsequent 6.x update to ctools (I'll be watching for it).

Thanks for your help with this.

Remove the isset as well -- that actually is part of the in array check (it's a notice check) and including it actually worsens the issue.

JUST have the user_access() check.

If this functions, we'll need a patch we can commit.

@wklyons When you try and make a patch for this, ping me, or ask in irc. I think this might be your first time making a patch. It can be tricky the first time. But it is a really good thing to be able to do. :) And check out the doc I linked to in #12.