Error when overwriting paragraphs-item.tpl.php

neessen commented 3 years ago

I have Paragraphs 1.x-1.1.0 and Entity Plus 1.x-1.0.13 installed, and getting an error when copying paragraphs-item.tpl.php from paragraphs module folder to the templates folder of the theme:

Error message
Notice: Undefined index: #view_mode in template_preprocess_entity_plus() (line 134 of /entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity_plus_type in template_preprocess_entity_plus() (line 135 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity in template_preprocess_entity_plus() (line 137 of /modules/entity_plus/theme/entity_plus.theme.inc).
Error: Call to a member function label() on null in entity_label() (line 673 of /core/modules/entity/entity.module).

laryn commented 3 years ago

Ah, maybe you can help me track this down, @neessen! I have this happening on one site only and haven't been able to figure out what is causing it. See here for a hack/workaround:

https://github.com/backdrop-contrib/entity_plus/issues/28

Maybe we can determine the conditions that cause this and fix in either Paragraphs or Entity Plus...

neessen commented 3 years ago

Hey Laryn Sure I will try :)

I have printet the paragraphs field in the node.tpl like this:

<?php print render($content['field_paragraphs']); ?>

As soon as I create the file "paragraphs-item.tpl.php" in the templates folder with the default markup it generates the error mentioned above. Content of file:

<div class="<?php print implode(' ', $classes); ?>"<?php print backdrop_attributes($attributes); ?>>
  <div class="content"<?php print backdrop_attributes($content_attributes); ?>>
    <?php print render($content); ?>
  </div>
</div>

The error is not present when this file is not in the templates folder.

Going to the entity.module file line 673 and changing to this:

function entity_label($entity_type, $entity) {
  if($entity){
    return $entity->label();
  }else{
    return;
  }
}

Generates a new error:

Error message
Notice: Undefined index: #view_mode in template_preprocess_entity_plus() (line 134 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity_plus_type in template_preprocess_entity_plus() (line 135 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity in template_preprocess_entity_plus() (line 137 of /modules/entity_plus/theme/entity_plus.theme.inc).
Error: Call to a member function uri() on null in entity_uri() (line 655 of /core/modules/entity/entity.module).

Changing line 655 to this

function entity_uri($entity_type, $entity) {
  if($entity){
    return $entity->uri();
  }else{
    return;
  }
}

Makes it work (it doesnt break the site with a php error), but gives me a pile of warnings:

Notice: Undefined index: #view_mode in template_preprocess_entity_plus() (line 134 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity_plus_type in template_preprocess_entity_plus() (line 135 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity in template_preprocess_entity_plus() (line 137 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: entity keys in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).
Notice: Trying to access array offset on value of type null in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).
Notice: Undefined index: #view_mode in template_preprocess_entity_plus() (line 134 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity_plus_type in template_preprocess_entity_plus() (line 135 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity in template_preprocess_entity_plus() (line 137 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: entity keys in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).
Notice: Trying to access array offset on value of type null in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).
Notice: Undefined index: #view_mode in template_preprocess_entity_plus() (line 134 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity_plus_type in template_preprocess_entity_plus() (line 135 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: #entity in template_preprocess_entity_plus() (line 137 of /modules/entity_plus/theme/entity_plus.theme.inc).
Notice: Undefined index: entity keys in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 353 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Trying to access array offset on value of type null in entity_extract_ids() (line 354 of /core/modules/entity/entity.module).
Notice: Undefined index: entity keys in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).
Notice: Trying to access array offset on value of type null in entity_plus_id() (line 164 of /modules/entity_plus/entity_plus.module).

But changing the core files is obviously not the best idea, and that is a lot of warnings, so I wonder if there is another workaround? :) It seems that for some reason it is not recognizing the paragraphs as entities for som reason since the entity_url and entity_label functions are breaking?

And also (I dont know if this is intended), but it seems that entity plus is somehow duplicating the markup in the paragraphs-item.tpl.php file.

If i give the .content div an extra class (TEST) the result is this:

<div class="field-item odd">
  <div class="entity-plus paragraphs-item-text-image paragraphs-item-full">
    <div class="content TEST">
      <div class="entity-plus">
        <div class="content TEST">
          <div class="field field-name-field-image field-type-image field-label-hidden">

And if I print some markup at the bottom of the paragraphs-item.tpl.php e.g.:

<?php $theme_path = backdrop_get_path('theme', config_get('system.core', 'theme_default')); ?>
<div class="<?php print implode(' ', $classes); ?>"<?php print backdrop_attributes($attributes); ?>>
  <div class="content TEST"<?php print backdrop_attributes($content_attributes); ?>>
    <?php print render($content); ?>
  </div>
</div>
TEST TEXT

It prints "TEST TEXT" twice below the paragraph content of each paragraph content.

neessen commented 3 years ago

I have been looking at this some more, and it seems like it is the "printing twice" part that is causing all the issues. I looked at the template_preprocess_entity_plus(&$variables) function in entity_plus.theme.inc, and that hook is being called twice which is causing all the issues. I dumped the $variables variable and it prints two arrays for each paragraph. I have pasted a simplified version below of that dump where I have removed some stuff to make it a bit more readable. It seems like the data is structured in two different ways which is causing the page to break and give warnings, because most of the variables used in the first 7 lines are not there in the second array:

    $variables['view_mode'] = $variables['elements']['#view_mode'];
    $entity_type = $variables['elements']['#entity_plus_type'];
    $variables['entity_type'] = $entity_type;
    $entity = $variables['elements']['#entity'];
    $variables[$entity_type] = $entity;
    $info = entity_get_info($entity_type);

    $variables['title'] = check_plain(entity_label($entity_type, $entity));

E.g. $variables['elements']['#entity'] does not exist in the second array, it would be something like: $variables['elements']['entity']['paragraphs_item'][p_id]['#entity'] because it is structured differently.

However this hook should only be called once so this shouldn't be an issue at all. I can't figure out why it is being called twice, but maybe the fact that the [theme_original_hook] is not the same could provide you with som answers to what is going on:

[theme_hook_original] => entity_plus [theme_hook_original] => paragraphs_item

As a temporary fix, i have wrapped the content of the template_preprocess_entity_plus() function in an if statement, but that does not fix the fact that the hook is being called twice which i dont think it should, so if you have any idea to why, or have to properly fix it, I would really appreciate it.

//Temp fix:
function template_preprocess_entity_plus(&$variables) {
  if(isset($variables['elements']['#entity'])){

  }
}

neessen commented 2 years ago

Hey @laryn Did you have a chance to look at this? :)

jenlampton commented 2 years ago

I'm experiencing the same problem. @neessen thank you for the temporary fix! I'm also seeing the entity_plus theme markup wrapping my paragraph item twice:

This is particularly problematic if, for example, your paragraph template split the area into two columns. When the template is applied twice, the second rendering splits the same area into two more columns, making everything half the size it should be.

I'm seeing the paragraph_item template being applied to the paragraph item, as expected, but then it's applied again when render is called on the $content variable inside the template.

I think the underlying problem is somewhere in entity_plus.

jenlampton commented 2 years ago

Okay, so I think I'm getting closer. At the time template_preprocess_paragraphs_item() is called, the paragraph has already been rendered. The rendered result (using the paragraph item template) has been stashed in the #children property of the $content array, and #printed has been set to true.

In spite of that, calling render() on this $content array still renders it again, but it also includes the pre-rendered version from #children.

Now to find out where that got pre-rendered... 1) It has NOT yet been rendered when it is assembled in paragraphs_field_formatter_view()

laryn commented 2 years ago

@neessen @jenlampton Sorry, I haven't been able to get back to this yet. Thanks to you both for digging around in it!

jenlampton commented 2 years ago

Bingo! I've filed a PR that fixes the double template problem on my site :)

I think the problem was that we were telling the paragraph item to use the paragraph_item template as a theme wrapper, but it already renders using that template due to entity_plus changing the theme_hook_suggestions in template_preprocess_entity_plus().

I don't like how entity_plus is both providing theme output, and then switching the output source to use something else, but that seems to be as intended. In a perfect world all it would do is render() and allow whatever provided the thing to specify it's own output. I didn't try to fix it though, that seems hard :)

laryn commented 2 years ago

@jenlampton I can confirm on a quick test that this seems to solve it on the site I was seeing this on, with no ill-effects noticed! Amazing.

@neessen Can you confirm as well?

laryn commented 2 years ago

I don't like how entity_plus is both providing theme output, and then switching the output source to use something else, but that seems to be as intended. In a perfect world all it would do is render() and allow whatever provided the thing to specify it's own output. I didn't try to fix it though, that seems hard :)

@jenlampton, I'm a maintainer on Entity Plus mainly because I needed to get a few fixes merged to make some other modules functional. My sense is there have been some gaps that need polish in a variety of places, so "as intended" may be used quite loosely here. Please feel free to open an issue in Entity Plus, however. There has been some traction on issues from others that are using it and it may open a conversation.

neessen commented 2 years ago

@jenlampton I can confirm it works as well :) I had to do a little work because some variables that were no longer available in paragraphs-item.tpl.php like $content['bundle'], but I managed to work around that, so yeah it looks like it is working.. Will this be included in next paragraph release then?

Thank you for your help on this @jenlampton and @laryn, I appreciate it :)

laryn commented 2 years ago

Thanks @jenlampton for the work to track this down and for the PR that fixes it! Thanks @neessen for reporting and helping to test. I've merged https://github.com/backdrop-contrib/paragraphs/pull/92 into 1.x so it will be in the next release of Paragraphs.

laryn commented 2 years ago

@jenlampton or @neessen I was testing this further prior to formal release and there may be an issue (or a further modification needed). Are either of you using template_preprocess_paragraphs_item on any sites that use Paragraphs? For example, the Classy Paragraphs module uses this hook to add classes to individual Paragraphs items. It seems that this fix may knock out the capability to use that hook for some reason.

jenlampton commented 2 years ago

I think I tried to use that function and failed, but then assumed that it wasn't supposed to work. I should have looked more closely!

neessen commented 2 years ago

@laryn yes I have used that hook on other sites, but not the one where I had this issue, so I didn't realise that it could have any impact. Have you had a chance to investigate why it has impact on that hook?

laryn commented 2 years ago

@neessen Not yet. But we'll need to figure it out (either in Paragraphs or Entity Plus) before the next release.

laryn commented 2 years ago

@neessen @jenlampton FYI I've added this line back in the alpha1 pre-release, realizing it's not a solution yet. Before we get to a full release we'll have to have solved this either here or in Entity Plus. But I want people to be able to start testing some of the other changes and I think a pre-release may make that easier for some folks.

https://github.com/backdrop-contrib/paragraphs/blob/1.x-1.x/paragraphs.field_formatter.inc#L124-L128

laryn commented 2 years ago

Interestingly, I realized I had just copied an older version of paragraphs-item.tpl.php with no changes on the site that I see this issue happening. I tried updating that to the latest version but still got this error. I deleted it completely and the site has no error. (Note that I do still have individualized templates, such as paragraphs-item--full-width-image.tpl.php, and they work fine).

argiepiano commented 2 years ago

I'm continuing the conversation here, from the related issue #28. My strong suspicion is that #28 and this issue are closely related and are the result of using paragraphs-item.tpl.php both as a theme_wrapper and as as a theme template as reported by @jenlampton in a comment above: https://github.com/backdrop-contrib/paragraphs/issues/91#issuecomment-917737703.

I can confirm that the PR she provided resolves the double use of the tpl, which takes care of the nested divs with the same classes (see graph in https://github.com/backdrop-contrib/entity_plus/issues/28#issuecomment-985975943)

I also found some Drupal commits that actually removed the use of that wrapper. See this commit. Drupal 7's current version of Paragraphs doesn't have that declaration of #theme_wrappers.

The drawback of removing the $element[$delta]['#theme_wrappers'] = array('paragraphs_item'); as in @jenlampton #92 is that template_preprocess_paragraphs_item never gets called. This is true both in D7 and Backdrop. Instead template_preprocess_entity_plus is used. In fact there was some conversation in Drupal about this problem (themers were confused by this), and someone proposed a patch to add the #theme_wrappers declaration. But that patch is not in the current version of D7's Paragraphs.

I think the next step is to try to reproduce this issue and also #28 after applying PR #91. @laryn, if you could help me reproduce the error that'd be great. I tried what you mentioned in #28 but no luck so far.

argiepiano commented 2 years ago

Trying to reproduce the error after applying #92 by following @laryn 's report, but still no warnings or errors. And the html markup is correct, without the nested divs with same classes

Interestingly, I realized I had just copied an older version of paragraphs-item.tpl.php with no changes on the site that I see this issue happening. I tried updating that to the latest version but still got this error. I deleted it completely and the site has no error. (Note that I do still have individualized templates, such as paragraphs-item--full-width-image.tpl.php, and they work fine).

herbdool commented 2 years ago

Maybe avoid using entity plus version of view and use it's build content instead. Or only do that within paragraphs?

argiepiano commented 2 years ago

Success!!! The modal is working fine now, no errors, and the template file is also in the theme folder. I will create a new issue and PR in a few minutes.

Basically what I did:

Apply PR #92 by @jenlampton (i.e. remove #theme_wrappers property from the paragraphs_item render array).@laryn , this means that I had to "reverse your reversal" here.
Apply the modification to the template to use array_classes done in #109
Moved much of the code from both template_preprocess_paragraphs_item() and paragraphs_preprocess() into paragraphs_preprocess_entity_plus() with an if statement to check that we are preprocessing the template for paragraphs-item

To summarize the source of the error: the use of #theme_wrappers and the resulting rewriting of the template file was causing all of this.

Preprocessing in Entity Plus is confusing. One would logically assume that the following preprocess hook would work: template_preprocess_paragraphs_item. But no. These preprocessing functions never get called (I suffered through this issue a few times myself!). Which is what led people to use #theme_wrappers (to force that hook to run, but caused these issues). Instead, you must use hook_preprocess_entity_plus and use if statements to check that you are preprocessing the right template/entity. Eventually this may be something we could fix within Entity Plus.

I will create a new issue a submit a PR with all of this in a few minutes.

herbdool commented 2 years ago

@argiepiano awesome! I haven't tested but that's great that you unraveled this knot.

laryn commented 2 years ago

HUGE shout out to @argiepiano who stepped in and crushed this one in https://github.com/backdrop-contrib/paragraphs/issues/113. Latest 1.x-1.2.0-alpha2 release should solve this while retaining use of template_preprocess_paragraphs_item().

I was a little over-eager in releasing this next alpha -- almost immediately afterwards I realized one minor tweak that should help with backwards compatibility: https://github.com/backdrop-contrib/paragraphs/commit/369b4a976c2e0746309904ec78b041860e870b16

I made note in the release notes but there may be some minor tweaks needed to custom implementations of template_preprocess_paragraphs_item() and customized template files (notably changing $classes to $classes_array) but when you get a chance could you test and see if this solves things on your end @jenlampton and @neessen?

neessen commented 2 years ago

Nice work - just upgraded to the 1.x-1.2.0-alpha2 version on a couple of my sites, with no problems so far.. :)

laryn commented 2 years ago

Great! I'm going to close this issue -- please reopen or open a new issue if something comes up with the latest release.

backdrop-contrib / paragraphs

Error when overwriting paragraphs-item.tpl.php #91