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.
Comment | File | Size | Author |
---|---|---|---|
#26 | ctools-1566112-26.patch | 2.1 KB | kristiaanvandeneynde |
Comments
Comment #1
MTecknology CreditAttribution: MTecknology commentedThis 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.
Comment #2
OnkelTem CreditAttribution: OnkelTem commentedThe 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)
Comment #3
OnkelTem CreditAttribution: OnkelTem commentedHere is another version which doesn't require dependency from Entity API, but works as expected only when Entity API is enabled.
Comment #4
OnkelTem CreditAttribution: OnkelTem commentedOne more version:
* Added comment describing the modification
* $view variable renamed to $content.
Comment #5
dawehnerSure 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.
Comment #6
OnkelTem CreditAttribution: OnkelTem commentedBUMP
Comment #7
tim.plunkettPossibly related: #1907800: Extra fields need the fully built entity
Trailing whitespace.
Split this over two lines
Comment #8
DamienMcKennaI fixed the two things tim.plunkett mentioned, but haven't actually tested or review the patch itself, just trying to keep things rolling.
Comment #9
maximpodorov CreditAttribution: maximpodorov commented#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.
Comment #10
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis 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
Comment #11
DamienMcKennaRerolled.
Comment #12
DamienMcKennaAdded some comments.
Comment #13
DamienMcKennaOops, this patch doesn't include the "version" directive to the ctools.info file that I'd added for local testing.
Comment #14
kristiaanvandeneyndeLooks 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.
Comment #15
maximpodorov CreditAttribution: maximpodorov commentedcomment.module doesn't define extra fields, so comments don't require specific handling.
Comment #16
kristiaanvandeneyndeThanks for clearing that up :)
Comment #17
DamienMcKennaI'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.
Comment #18
kristiaanvandeneyndeWell 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?
Comment #19
DamienMcKennaI 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).
Comment #21
kristiaanvandeneyndePerhaps 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:
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.
Comment #23
DamienMcKenna@kristiaanvandeneynde: I'd be interested to know why you avoid Panelizer, though obviously it's off-topic for this issue.
Comment #24
kristiaanvandeneynde@DamienMcKenna I'll send a PM because it is completely off-topic indeed :)
Comment #25
japerrySo 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!
Comment #26
kristiaanvandeneyndeDone, code should explain what's going on.
Comment #27
MichelleI'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.
Comment #28
kristiaanvandeneyndeYou'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:
Comment #30
OnkelTem CreditAttribution: OnkelTem commentedGood 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.
Comment #31
OnkelTem CreditAttribution: OnkelTem commented