Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Letharion’s picture

Assigned: Unassigned » merlinofchaos
FileSize
1.58 KB
5.88 KB

Export of the failing variant and the site template

Letharion’s picture

Assigned: merlinofchaos » Letharion
Letharion’s picture

Note to self:
Compare call stacks between a node_view and node_edit callback, and each PE functions format of return value.
Something that currently does return array; should probably be doing return array['content'];

pontus_nilsson’s picture

More information...

I had a look at the watchdog after hitting the problem Letharion describes.

The watchdog says: "Theme key "panels_edit_display_form" not found."
Refferer: admin/structure/pages/nojs/operation/node_view/handlers/node_view_panel_context/content?render=overlay

Letharion’s picture

I see the theme key not found error as well.
Someone else has reported that specific problem with just Panels here #1116522: Warning: Theme key "panels_edit_display_form" not found.

bryancasler’s picture

subscribe

pontus_nilsson’s picture

Priority: Normal » Critical

Marking this as critical, since it is a show stopper. I would like to help out on this issue, but I need help to understand what is executed and in what order to isolate the problem.

entrigan’s picture

Assigned: Letharion » Unassigned

I can confirm this issue. I tried tracking down the source, but it revealed little.

Some observations:
1) panels_everywhere_page_content_content_type_render(() is not getting called
2) preview on panels page shows nothing
3) The problem happens only when the node edit page is enabled and there is an active variant with active selection criteria
4) Unlike on pages that do work, panels_everywhere_ctools_render_alter() is being called for the node edit page

Hmmmmmmmm

entrigan’s picture

It appears that something is wrong in or around the node_edit_form context plugin. Changing the name element from 'node_edit' to 'entity_id:node' makes the page display correctly:

function page_manager_node_edit_get_arguments($task, $subtask_id) {
  return array(
    array(
      'keyword' => 'node',
      'identifier' => t('Node being edited'),
      'id' => 1,
      //'name'=>'node_edit
      'name' => 'entity_id:node',
      'settings' => array(),
    ),
  );
}

However this of course removes the node_edit_form context which means all of the form elements are not available.

Prodding a little bit more, I see that in ctools_context_create_node_edit_form(), $empty is set to true, and a unfulfilled context object is being returned.

merlinofchaos’s picture

Not using Panels Everywhere, I've got node_edit working -- but if you're using an older version of CTools than the -beta then node_edit might be still broken.

entrigan’s picture

The problem is specifically with Panels Everywhere active (and with a page template variant active). I reproduced the problem with the following three setups:

1. Dev versions of Ctools, Panels, and Panels Everywhere on Vanilla Drupal 7.2
2. Latest stable versions of Ctools, Panels, and Panels Everywhere on Vanilla Drupal 7.2
3. Alpha 3 of Ctools, Alpha2 of Panels, and Alpha 1 of Panels Everywhere on Vanilla Drupal 7.2

entrigan’s picture

I attempted to do more debugging. I have somewhat limited skills with php and debugging so bear with me:

It appears the the function page_manager_node_edit_get_arguments() gets called twice, and that on the second calling of the function, the next statement that returns an array will instead output Array (or if there is a devel dpm/kpr statement, it will output that) to the page and stop running.

entrigan’s picture

The problem is with the node context getting build by the site_template task. Commenting out panels_everywhere_site_template_add_context($contexts, $node, t('Node being viewed'), 'node', 'node'); in the function panels_everywhere_site_template_get_base_contexts() makes the node edit page work again.

This is of course has the side affect of making the node context no longer available to the site template : ( Not sure how to fix it but at least we know where the source of the problem is.

Thanks for pointing me in the right direction on IRC Merlin.

entrigan’s picture

Merlin suggested the solution might be to cache the context in the node_edit_form context plugin. Unfortunately the node_edit_form plugin seems to only get called once, and hence caching had no affect.

However I did discover in all of this that the node context was getting called twice, and I discovered a fix: Removing 'node' from the ctools_context array argument in the node_edit_form context:
In the function

 function ctools_context_create_node_edit_form($empty, $node = NULL, $conf = FALSE) {
  static $created;

  //OLD LINE
  //$context = new ctools_context(array('form', 'node_edit', 'node_form', 'node', 'node_edit_form'));
  
  //NEW LINE
  $context = new ctools_context(array('form', 'node_edit', 'node_form', 'node_edit_form'));
  $context->plugin = 'node_edit_form';

This is all a bit over my head at the moment, but so far testing has not revealed any ill side effects (even with panels everywhere disabled).

Thoughts? If this is a reasonable fix I will roll a patch.

merlinofchaos’s picture

Um. Sure if that fixes it I'm all for it.

It's possible that something treats it as a node context and squashes the form somewhere? I dunno why that would be.

entrigan’s picture

Project: Panels Everywhere » Chaos Tool Suite (ctools)
Priority: Critical » Major
Status: Active » Needs review
FileSize
634 bytes

In that case, changing projects to ctools, and attaching patch.

merlinofchaos’s picture

Status: Needs review » Fixed

Committed!

Itangalo’s picture

(I can confirm that it is now possible to have node add forms in Panels Everywhere. Wohoo!)

jthomasbailey’s picture

Was this committed to 6? Because it's not working.

jthomasbailey’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Fixed » Patch (to be ported)

Hey Merlin I don't think this was committed to 6. The patch works, though.

merlinofchaos’s picture

Status: Patch (to be ported) » Needs review

If the patch works for 6.x as is then let's mark it needs review so I find it. :)

Letharion’s picture

@merlinofchaos
I have verified that the patch as it is applies to 6 as well, but I don't have a PE site currently on D6, so I haven't verified it's effectives. It's my understanding however that this has been confirmed by #19 in #965148: node add/edit and taxonomy pages are not rendered when using site template
You may or may not want to wait a bit to see if someone re-opens that bug.

Itangalo’s picture

I just realized/experienced that an unfortunate side effect of this patch is that Page manager loses all knowledge about the content type, and thus which fields are available in the form.

I first thought that I should try creating a CTools relationship plugin, node form --> node, but then I realized that it won't help very much. The edit fields are of course found through the node form context data, not the node display data.

Don't really know how to approach this problem.

entrigan’s picture

Ok. Maybe caching was the appropriate way to solve this. When I tried caching the node object in the node context function it did not solve the issue, so this will likely have to be done higher up in the pipeline.

Astma1’s picture

I had the same Problem.
The patch seemed to fix this issue, but...
Patching the "node_edit_form.inc" ends up with not being able to create a selection rule depending on the node-type for the concerned node add/edit form variant.

Itangalo’s picture

Status: Needs review » Needs work
FileSize
625 bytes

I have investigated this issue a bit more, and found something really strange.

When un-applying the patch from #16, I get "array" on the node add form when using Bartik. However, it seems to be working fine with I use Seven for adding/editing nodes. This makes me think that the issue is theme related, and that the ideas in #4 is a good start for solving them.

Attached is a patch to get back the node context for the forms.

entrigan’s picture

@Itangalo so there is no problem when using seven even with Panels Everywhere enabled and a variant active?

Also of note, the same issue came up when trying to do the analogous setup for the user edit page.

Itangalo’s picture

@entrigan: I might be mistaking, when I give it second (and third) thoughts. Won't be able to check this out for some more days, though. Sorry. :-/

entrigan’s picture

I recently ran into a similar issue with Array bring output. I think it was because I was running render() on a render array twice. Perhaps the issue here is similar where render is getting processed twice?

(I have not followed up on this at all, this is really a note to self for future investigation)

emattias’s picture

I was able to get it working by checking if $variables['page']['#children'] is an array in preprocess_html and if so render that array:


function brsv_preprocess_html(&$variables){
  
  if(is_array($variables['page']['#children'])) {
    $variables['page']['#children'] = render($variables['page']['#children']);
  }
}

emattias’s picture

FileSize
659 bytes

The problem now is that this results in one form tag around the whole body(panels everywhere panel) and another around the content pane(normal content panel) which is invalid HTML and breaks completely in IE8.

I've investigated this and traced it down to this code in panels.module:


function panels_render_display(&$display, $renderer = NULL) {
  ctools_include('plugins', 'panels');
  ctools_include('context');

  if (!empty($display->context)) {
    if ($form_context = ctools_context_get_form($display->context)) {
      $form_context->form['#theme'] = 'panels_render_display_form';
      $form_context->form['#display'] = &$display;
      return $form_context->form;
    }
  }
  return $display->render($renderer);
}

The if (!empty($display->context)) { statement checks if this is panel has a form context and if so it renders it as a form.

I'm unsure how to proceed with this problem. I have come up with these solutions:

1. Add an option to stop panels from completely wrapping any panel in a form tag and instead let you control which region(and possibly a single pane) is wrapped in the form tag. I have attached an example of this that I got working when I commented out the if (!empty($display->context)) { statement(so that it doesnt wrap the whole panel in form tag). This solution allow you to have several forms in one form panel(in different regions/panes).

2. Somehow detect that the current panel is a panels everywhere panel and never render it as a form.(Results on only one form tag around the content panel).

emattias’s picture

Here's a patch that stops site template from being rendered as a form which seams to fix this whole issue. To be clear, with this patch my workaround in #30 is unnecessary.

emattias’s picture

Status: Needs work » Needs review

Change status of the issue

Status: Needs review » Needs work

The last submitted patch, dont-render-site-template-as-form-1139918.patch, failed testing.

emattias’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, dont-render-site-template-as-form-1139918.patch, failed testing.

merlinofchaos’s picture

Status: Needs work » Needs review

Yes yes, testbot.

pixelmord’s picture

Hi,

I'm experiencing the same problem, also when using seven theme that goes away.

I'm using the
panels 7.x-3.x-dev (2011-Nov-03)

Automatic apply of the dont-render-site-template-as-form-1139918.patch failed.
I applied the patch manually and my first tests show, that it works so far also in my custom theme.

I attached the patch I created.

shadcn’s picture

Patch in #38 worked for me. (panels, + panels everywhere). Thanks @pixelmord

arnested’s picture

FileSize
1.68 KB

The patch from #38 works for me as well although displays some notices:

Notice: Undefined property: panels_display::$task_name in panels_render_display() (line 1025 of .../sites/all/modules/contrib/panels/panels.module).

Improved patch attached.

HnLn’s picture

patch in #40 didn't work for me in d7, #38 did and seems to have fixed the problem

merlinofchaos’s picture

Okay, this is a bug in Panels Everywhere with a patch for Panels and it's in the CTools module queue. That's exciting.

Having to hardcode the site_template check in Panels is wrong. Panels Everywhere should be able to do something about the form context so that Panels doesn't screw it up. I think maybe it should just refuse to inherit form contexts or un-formify them.

This patch will at least get people going, but I'm going to move it to PE.

merlinofchaos’s picture

Project: Chaos Tool Suite (ctools) » Panels Everywhere
merlinofchaos’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev

Also the patch is against D7 but the issue is against D6. :(

MiSc’s picture

FileSize
1.68 KB

Rewrote patch to make it work.

drupov’s picture

The patch from #45 solves the issue with page content being wrapped in a form tag, when using a node add/edit variant together with Panels Everywhere (see issue https://drupal.org/node/1736368)

markus_petrux’s picture

This is PE queue with a patch for Panels?

PS: Marked another issue as a dup. See #2123869: Double wrap content in <form> tag on user edit page

drupov’s picture

Issue summary: View changes

Hi,

#46 this issue appears only when you use node add/edit on a site that uses Panels Everywhere as site template. So it's kinda philosophical question where it belongs :)

More important: the patch from #45 doesn't apply anymore with the recently released panels 3.4:

patching file panels.module
Hunk #1 FAILED at 1023.
Hunk #2 succeeded at 1099 with fuzz 1 (offset 33 lines).
1 out of 2 hunks FAILED -- saving rejects to file panels.module.rej
patching file plugins/task_handlers/panel_context.inc
Hunk #1 FAILED at 268.
1 out of 1 hunk FAILED -- saving rejects to file plugins/task_handlers/panel_context.inc.rej

I guess the failed parts have changed a lot during the dev phase of panels between 3.3 and 3.4, but I can't really figure out what the problem here is. Maybe someone has a clearer idea?

drupov’s picture

Status: Needs review » Needs work

Changing status.

pontus_nilsson’s picture

I think this is fixed in the dev branch of PE. Could you try using 7.x-1.x-dev instead of 7.x-1.0-rc1?

One of our site uses the node add/edit variant without problems and I checked the makefile and we point to a commit in the dev-branch instead of using 7.x-1.0-rc1.

drupov’s picture

Just tried it. This is not solved by using the latest dev of panels_everywhere with panels 3.4.

roberttstephens’s picture

This issue was not fixed when using the latest panels everywhere and panels code.

I rerolled the patch from comment 45 on the latest panels 7.x-3.x branch, however the first portion of that patch was unnecessary.

I understand that the "right way" may be to fix this in panels everywhere, but some of us need working sites in the meantime.

drupov’s picture

Confirming the patch from #52 works fine with panels 7.x-3.4+2-dev, ctools 7.x-1.4 and panels_everywhere 7.x-1.0-rc1.

Leaving this at "needs work" as it's not clear where the actual fix should appear (PE or panels). So far I am really happy to have this solution.

Thanks @roberttstephens!

rackberg’s picture

DamienMcKenna’s picture

Priority: Major » Normal
Status: Needs work » Closed (cannot reproduce)
Issue tags: +SprintWeekend2015

I'm unable to reproduce any problems with the current -dev releases of CTools, Panels and Panels_Everywhere.

If you are still experiencing the problem with these module versions, please re-open this issue and provide specific steps to reproduce the bug. Thanks.

func0der’s picture

This is still a BIG problem.

I do not know how can not re-produce this. I got this first thing after setting up a new Drupal 7.

Anyway. Steps to re-produce this:

  1. Setup a new and fresh Drupal 7 BASIS installation.
  2. Install Panels Everywhere via drush. All dependencies should be fetched and enabled by it.


    drush en panels_everywhere
  3. Login as admin, go to "Structure" -> "Pages". Enable the "Site template" and edit it. Create a new variation. No special selection rules or contexts. They all will be applied by the code later. Chose a default layout. I chose the "Builder" thing, but that does not matter, I guess. Put the "Page Elements" -> "Page Content" in the site template.
  4. Go again to "Structure" -> "Pages" and enable the "Node add/edit form" page. Edit it. Create a new variation with no special settings and in the content you put a "Form" -> "General Form" pane and chose "Node being edited" in the settings.
  5. Give the guests the permission to create a new article.
  6. Open up a private session in your browser where you are NOT logged in or at least not as an admin. Go to "/node/add/article" and check the source code ;)

The structure should be like this:

body
    -> div#skip-link
    -> form#article-node-form
        -> div
            -> d.panel-display panel-1col clearfix
            ...

There you go. I hope this helps to fix this problem in Panels Everywhere and not in other modules that should not even care for Panels Everywhere.

I hope that helps you :)

func0der

func0der’s picture

Priority: Normal » Major
Status: Closed (cannot reproduce) » Needs work
MiSc’s picture

Priority: Major » Normal

Can not see how this can be a major issue, and I really don't understand what we should check in the source code for - could you please add what the result is, and what it should be?

func0der’s picture

Priority: Normal » Major

It breaks the functionality of EVERY form on the website if we are on public node add/edit pages.
So I think this is pretty major.

I do not know what more to say to the result that IS there than I already did.
The result is simply that the whole page (except for the skip link) is surrended by the form of the node add-/editing.

The SHOULD be is simple: Only surround the real form. If you want to see that in code, apply patch from #52 and compare the results.

It is easy to see.

MiSc’s picture

Please attach what errors you get, to tell somebody too look an the error without posting it is not very collaborative. If we help each other out, problems could easier be solved.

DamienMcKenna’s picture

Title: Node add/edit-form fails to render with Panels Everywhere » Node add/edit-form fails to render when using Panels Everywhere
Project: Panels Everywhere » Panels
Version: 7.x-1.x-dev » 7.x-3.x-dev
Issue tags: +Panels Everywhere

Ok, yes, I've reproduced the error, the problem occurs as described by func0der in #56 and the patch from #52 appears to fix the problem.

DamienMcKenna’s picture

DamienMcKenna’s picture

Project: Panels » Panels Everywhere
Version: 7.x-3.x-dev » 7.x-1.x-dev

Moving this back to the Panels Everywhere queue because I think I have a possible solution.

DamienMcKenna’s picture

Taking a cue from merlinofchaos in comment #42, this blanks out the node context if it's actually a form. I'm not completely happy with this, but merlinofchaos didn't like the alternative - hardcoding Panels to be aware of Panels Everywhere's site_template task.

drupov’s picture

I just tried the patch from #64 and it works.

Thanks @DamienMcKenna

DamienMcKenna’s picture

@drupov: Excellent, thanks for the feedback. I still need to discuss it with japerry first, but I'm happy there's a patch that could resolve the problem from this module's side, albeit kludgily so.

4alldigital’s picture

I've tried patch #64 but not sure its applicable to my scenario of the issue.

I have /user/%user/edit enabled from the https://www.drupal.org/project/user_pages module. After applying path #64 it didn't fix the fact 95% of the page markup was wrapped in the general form [user edit form].

After debugging line 97 of site_template.inc I confirmed $account->data->logged_in_user is not set (see attached screenshot dpm()) so $account = ctools_context_create_empty('user'); does not get called.

When putting that statement outside of the isset() function it does fix the issues, but I don't know enough about panels everywhere to help much more than this.

EDIT:

could you not make this line [97 of site_template.inc ] :

if (isset($account->data->logged_in_user) || in_array('authenticated user', $account->data->roles)) {

if you're just checking for logged in user status??

4alldigital’s picture

sorry to spam the issue, but just confirmed neither solution about have 100% fixed this as the form is still wrapping around panel div's (see attached)

markup is :

<form class="user-profile-form" enctype="multipart/form-data" action="/user/541/edit" method="post" id="user-profile-form" accept-charset="UTF-8">
  <div>
    <div class="l-content">
    <div class="l-content--inner">
      <div class="panel-pane pane-form">
        <div id="edit-account" class="form-wrapper"><div class="form-item form-type-textfield form-item-name">

markup should be :

<div class="l-content">
  <div class="l-content--inner">
    <div class="panel-pane pane-form">
      <form class="user-profile-form" enctype="multipart/form-data" action="/user/541/edit" method="post" id="user-profile-form" accept-charset="UTF-8">
        <div>
          <div id="edit-account" class="form-wrapper"><div class="form-item form-type-textfield form-item-name">

happy to help if anyone wants me to test anything...

DamienMcKenna’s picture

Going to need to add tests for this.

DamienMcKenna’s picture

DamienMcKenna’s picture

Now that we have some tests I can expand upon them to work out what's going on here.

IRuslan’s picture

#64 works for me. I would like to set RTBC, but not sure if tests are required.

drupov’s picture

#64 works on a new site for me again.

plopesc’s picture

Hello all,

I'm having the same situation described by @4alldigital, and tested his suggestion, which worked for me.

So here I'm posting a new patch adding this extra feature to patch in #64.

Thanks

plopesc’s picture

FileSize
828 bytes
1.84 KB

Updating patch to avoid PHP notice

SocialNicheGuru’s picture