The ctools_entity_field_extra_content_type plugin makes assumptions that anything listed in hook_field_extra_fields() is added to an entity via hook_ENTITYTYPE_view() (i.e. hook_file_view()) and hook_entity_view() and never via the base function that renders an entity, e.g. node_view(), or in this specific case file_view(). I think this also seems to miss the fact that extra fields could also be added via the alter hooks, e.g. hook_file_view_alter() and hook_entity_view_alter() and doesn't invoke those as well.

This causes problems because the file_view() function from file_entity that 'renders' a file entity outputs the file data in $file->content['file']. The 'file' content is also listed in file_entity_field_extra_fields() since we want the user to be able to re-order it. But this means that when the user selects the 'File' content under the 'File' category when adding Panels content, it will never be able to render because file_view() is not used to render the entity.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MTecknology’s picture

This seems to also creep up with the eva module when trying to add the view to a page (panel). The view will attach just fine in the node itself but will never render inside of a panel. Sounds like this is the exact same issue.

OnkelTem’s picture

Status: Active » Needs review
FileSize
1.55 KB

The patch replaces two module_invoke's with entity_view() function from Entity API. This introduces dependency from Entity API module what I personally doesn't consider as a sin, since Entity API is a de facto standard on D7 websites.

(This patch doesn't includes ctools.info`s dependencies modification yet)

OnkelTem’s picture

FileSize
1.58 KB

Here is another version which doesn't require dependency from Entity API, but works as expected only when Entity API is enabled.

OnkelTem’s picture

FileSize
2.04 KB

One more version:

* Added comment describing the modification
* $view variable renamed to $content.

dawehner’s picture

Sure ctools should, at least for now, not require entity module, especially because updates of ctools could cause problems,
but to be realistic, every site which have the problem mentioned above does have heavy usage of entities and so
will for a big percentage have entity module installed.

Additional this also does not add a huge amount of code redundant code, so i think this is okay, even entity api module could in fact reimplement this content type.

OnkelTem’s picture

BUMP

tim.plunkett’s picture

Possibly related: #1907800: Extra fields need the fully built entity

+++ b/plugins/content_types/entity_context/entity_field_extra.incundefined
@@ -102,8 +102,19 @@ function ctools_entity_field_extra_content_type_render($subtype, $conf, $panel_a
+  // The best way to get entity content is to use entity_view() function ¶
+  // from Entity API module. For example commerce_product entities (from Drupal ¶
+  // Commerce) or file entities can only be "viewed" (have content built)
+  // only via entity_view(). But since CTools doesn't depend on Entity API
+  // we need to make sure it is available. Otherwise we fallback to old-style ¶

Trailing whitespace.

+++ b/plugins/content_types/entity_context/entity_field_extra.incundefined
@@ -102,8 +102,19 @@ function ctools_entity_field_extra_content_type_render($subtype, $conf, $panel_a
+  } else {

Split this over two lines

DamienMcKenna’s picture

Issue summary: View changes
FileSize
1.15 KB

I fixed the two things tim.plunkett mentioned, but haven't actually tested or review the patch itself, just trying to keep things rolling.

maximpodorov’s picture

#1947680: Extra field renderer is completely broken tries to fix this by listing known function names. For unknown entities, I think, the approach this patch provides, can be used.

SocialNicheGuru’s picture

this patch no longer applies to the newest version of ctools.

I tried to apply to the latest dev version 3/4/14:
git apply ctools-n1566112-8.patch
error: patch failed: plugins/content_types/entity_context/entity_field_extra.inc:102
error: plugins/content_types/entity_context/entity_field_extra.inc: patch does not apply
patch -p1 < ctools-n1566112-8.patch
patching file plugins/content_types/entity_context/entity_field_extra.inc
Hunk #1 FAILED at 102.
1 out of 1 hunk FAILED -- saving rejects to file plugins/content_types/entity_context/entity_field_extra.inc.rej

DamienMcKenna’s picture

FileSize
1.13 KB

Rerolled.

DamienMcKenna’s picture

Title: ctools_entity_field_extra_content_type always assumes extra fields are avaiable from hooks only » ctools_entity_field_extra_content_type always assumes extra fields are only available from hooks
FileSize
1.49 KB

Added some comments.

DamienMcKenna’s picture

FileSize
1.23 KB

Oops, this patch doesn't include the "version" directive to the ctools.info file that I'd added for local testing.

kristiaanvandeneynde’s picture

Looks good to me, although the fix from #1947680: Extra field renderer is completely broken is kind of 'ugly' compared to this one. If we could rely on people having Entity API enabled, the solution would look a lot cleaner.

Side question: What happens to comments (both with or without Entity API enabled)? They fall outside the scope of the solution from #1947680: Extra field renderer is completely broken.

maximpodorov’s picture

comment.module doesn't define extra fields, so comments don't require specific handling.

kristiaanvandeneynde’s picture

Thanks for clearing that up :)

DamienMcKenna’s picture

I've found this to be a horrible performance hit and personally recommend building custom content type panes to handle the individual fields that are needed.

kristiaanvandeneynde’s picture

Well if you want to go about things correctly, you have to fully 'view' the entity.
Out of curiosity: How big is the performance hit and which part is the worst offender?

DamienMcKenna’s picture

I suppose the performance problem could be worked around, but if you're using e.g. Panelizer to manage your displays and render an extra field using a Panelized view mode you'd end up with a lot of wasted code being executed (which was the problem we ran into).

kristiaanvandeneynde’s picture

Perhaps this patch should be re-rolled to work with the commit from #1947680: Extra field renderer is completely broken?

I'd suggest the following order:

  1. Entity API
  2. Core entities
  3. Fallback

Semi off-topic regarding the performance hit: Unless people use Panelizer, it shouldn't be that much of a hit. I personally try to avoid that module wherever I can. Panels works great, though.

DamienMcKenna’s picture

@kristiaanvandeneynde: I'd be interested to know why you avoid Panelizer, though obviously it's off-topic for this issue.

kristiaanvandeneynde’s picture

@DamienMcKenna I'll send a PM because it is completely off-topic indeed :)

japerry’s picture

Status: Needs review » Needs work

So this probably needs work after committing the code to #1947680: Extra field renderer is completely broken -- anyone have time to tackle this before 1.6? I'll make time to review!

kristiaanvandeneynde’s picture

Status: Needs work » Needs review
FileSize
2.1 KB

Done, code should explain what's going on.

Michelle’s picture

I'm trying to understand how to reproduce this. I added a file context and then added panels content with the 'File' content under the 'File' category as the OP suggested. Nothing renders when I view the page, so I thought I had reproduced the problem. But applying the patch didn't change anything. At this point, I don't know if the patch isn't working or if I am reproducing it wrong. If someone could clarify, I'd be happy to test.

kristiaanvandeneynde’s picture

You'd have to register an extra field through code and then add that extra field in either the 'base' view function of the entity you added it to or in hook_entity_view_alter().

Here's a list of extra fields on File Entity, try testing one of those:

      $info['file'][$type]['form']['filename'] = array(
        'label' => t('File name'),
        'description' => t('File name'),
        'weight' => -10,
      );
      $info['file'][$type]['form']['preview'] = array(
        'label' => t('File'),
        'description' => t('File preview'),
        'weight' => -5,
      );
      $info['file'][$type]['display']['file'] = array(
        'label' => t('File'),
        'description' => t('File display'),
        'weight' => 0,
      );

OnkelTem’s picture

Status: Needs review » Closed (duplicate)
Parent issue: » #1947680: Extra field renderer is completely broken

Good work all. We had to mark this as duplicate of #1947680: Extra field renderer is completely broken (or vice versa) but instead have been working on both.
But since the patch from #1947680: Extra field renderer is completely broken is committed, let's finish with this one.

OnkelTem’s picture