Currently node_view.inc does not execute the normal Drupal hooks, e.g. hook_node_view and hook_entity_view. In order for several other modules to work correctly, e.g. Metatag, these hooks must be executed.
Original request:
Currently the node_content.inc plugin uses node_invoke($node, 'view') to trigger node_view(), which means that several key node-related and entity-related hooks do not get triggered unless that pane is added to the page. Several modules, include Metatag, depend on entity hooks triggering on entity view pages, which won't happen. I suggest moving the entity/node view logic to node_view.inc, and trim the node_content.inc logic to just building the final output.
Comment | File | Size | Author |
---|---|---|---|
#13 | ctools-n1760384-13-d7.patch | 2.12 KB | meba |
#12 | ctools-n1760384-12-d7.patch | 2.12 KB | DamienMcKenna |
#11 | ctools-n1760384-11-d7.patch | 2.12 KB | DamienMcKenna |
#10 | ctools-n1760384-10-d7.patch | 2.01 KB | DamienMcKenna |
#9 | ctools-n1760384-9-d7.patch | 2.08 KB | DamienMcKenna |
Comments
Comment #1
merlinofchaos CreditAttribution: merlinofchaos commentedThat doesn't work; the node_invoke call can do a lot of modifications to the node that are necessary for that rendering, so simply moving it is not an option.
Comment #2
DamienMcKennaComment #3
DamienMcKennaI've updated the title to clarify the task.
Comment #4
DamienMcKennaThis is a Page Manager thing.
Comment #5
DamienMcKennaRelated:
Comment #6
DamienMcKennaComment #7
DamienMcKennaThis patch clones most of the code from node_view_multiple() in order to trigger the appropriate hooks.
Comment #8
DamienMcKennaRelated Metatag issue: #1363476: Panels integration - ensure meta tags work OOTB on entity pages
Comment #9
DamienMcKennaA slight variation of #7 that moves the generic core snippets to the top of the function and the CTools custom code underneath.
Comment #10
DamienMcKennaA huge simplification of the code by just running node_page_view() at the beginning, and storing its output for later in case it's needed.
Comment #11
DamienMcKennaSome updated comments, functionally it is identical to #10.
Comment #12
DamienMcKennaA small tweak to the last patch that removes the $function variable as it is no longer needed.
Comment #13
meba CreditAttribution: meba commentedThe patch did not apply anymore. I rerolled it and also confirmed that once applied Meta Tags start showing on Panel pages that are not using full rendered output
Comment #14
DamienMcKennaThis needs some additional work, it should also trigger the page title handling.
Comment #15
DamienMcKennaNever mind, I think I found the solution for the page titles in #1732538: Page title pattern ignored. Pushing this back for review.
Comment #16
Peter Törnstrand CreditAttribution: Peter Törnstrand commentedThis patch (from #13) did not have any effect on my setup. The meta tags (from metatag.module) did not show up on my node.
Meta tags 7.x-1.0-alpha8
Ctools 7.x-1.2
Panels 7.x-3.0
Panels everywhere 7.x-1.0-rc1
Comment #17
DamienMcKenna@Peter: Please try the -dev version of Metatag.
Comment #18
DamienMcKenna@Peter: also note that currently the page title is not overridden correctly, I'm working separately to resolve that (#1732538: Page title pattern ignored), so please check the description and other tags.
Comment #19
philsward CreditAttribution: philsward commentedPatch from #13 gave me the following error:
Redownloading 1.x-dev from 2012-Oct-11 makes the error go away, so definitely something with the patch...
Hope to see something working between this and metatags soon : )
Comment #20
philsward CreditAttribution: philsward commentedOh, I forgot to mention that I'm using the metatag-1.x-dev-[2012-Oct-16] and I'm still not having any luck with the metatag description after applying patch #13 : (
Comment #21
thelmer CreditAttribution: thelmer commentedPatch #13 don't work with :
ctools (7.x-1.2) + metatag 7.x-1.0-beta1
or latest dev versions of ctools+metatag
Also the patch breaks conditionals in webform 7.x-4.0-alpha6
The div id's are changed from : id="xxx-1" to: id="x--2-1"
Seems like node_page_view($node) are invoked twice
Comment #22
DamienMcKenna@Thelmer: when you say it "didn't work", what meta tags did you check and entity page were you viewing (taxonomy term, node, etc)?
Comment #23
thelmer CreditAttribution: thelmer commentedHi Damien
I'm trying with node pages displayed with panels.
I have only been looking at the description meta data.
After some more testing I have seen that it actually works with 1.2+beta1 on regular nodes but not on the one I use for the front page. For now I can set the description here with the global metadata settings for the frontpage.
I still see the prblem with webform conditionals.
Comment #24
DamienMcKenna@thelmer: sorry, I got mixed up between this and another issue.
I'll have to look into Webform.
Comment #25
liquidcms CreditAttribution: liquidcms commentedpatch in #13 worked for me to add metatag keywords on to a node overridden by panels.. thanks a lot guys.
Comment #26
Lukas von BlarerWorks for me as well.
Comment #27
othermachines CreditAttribution: othermachines commentedPatch in #13 works for me in that it adds meta tags to a node overridden with a panel. Thanks!
Meta tags 7.x-1.0-alpha8
Ctools 7.x-1.2
Panels 7.x-3.3
Comment #28
othermachines CreditAttribution: othermachines commented@DamienMcKenna:
After updating I no longer seem to need this patch anymore. Everything works fine.
Drupal 7.22 (from 7.16)
ctools 7.x-1.3 (from 7.x-1.2)
metatag 7.x-1.0-beta5 (from 7.x-1.0-alpha8)
panels 7.x-3.3 (not updated)
Comment #29
DamienMcKennaThe patch is no longer strictly needed as I built some workarounds in Metatag.
That said, I still think the change to CTools is worthwhile as it should be triggering the correct hooks.
Comment #30
caschbre CreditAttribution: caschbre commentedI have a project with logic in hook_node_view that is needed and not being executed for panel pages. I'm going to give this patch a shot to see if it solves the issue.
Comment #30.0
caschbre CreditAttribution: caschbre commentedRewrote the description.
Comment #31
justafishPatch in #12 applied and worked great. Thanks @DamienMcKenna!
Comment #32
japerryWhile #13 works as well I'd like to have a few more eyes on this before I commit it since it makes a few changes and the patch is relatively old. Marking needs review.
Comment #33
joelpittetI've been using this in production for a number of months and it's been working well.
Comment #35
dillix CreditAttribution: dillix commented#13 works great on few sites with latest Drupal 7.34, ctools and Metatag 7.x-1.4, please commit it. hook_node_view now executes for panel pages with this patch.
Here is log:
Comment #36
mrjmd CreditAttribution: mrjmd commentedComment #37
japerryCoolness, committed.
Comment #40
rjacobs CreditAttribution: rjacobs commentedI'm not quite sure if this specific issue should be re-opened or a new one started, but I suspect there may be some problems with the solution that was committed. It seems that this fix introduces situations where node-related hooks, such as hook_node_view(), get fired twice for the same $node entity variable. It seems that this could be a problem if related hook implementations make certain kinds of alterations.
The case that I've been debugging, and that lead me to this issue, involves the addition of a book node within a panels pane (added as a "rendered node"). With ctools 1.5 all was fine, but now with ctools 1.6 we are getting a barrage of php notices triggered from node_page_title(). As best I can tell, this is due to the following:
$default_output = node_page_view($node);
line that was added from this patch. This triggers all node-related hooks, such as hook_node_view() as intended even though the returned output is never used.Commenting out the
$default_output = node_page_view($node);
line that was added to page_manager_node_view_page() in this issue's commit makes the problem go away.Is it ok to assume that some node hooks, like hook_node_view(), can be safely triggered more than once on the same $node object? I'm sure the specific problem we encountered is somewhat case-specific but it would seem that there is a decent potential for other issues here (with this example being only demonstrative). It also seems like a performance concern to be redundantly building and rendering the same node twice, but only displaying it once.
In light of all this, please let me know if this should be re-opened or a new issue started.
Comment #41
DamienMcKenna@rjacobs: Please open a new issue. Also, oops :-\
Comment #42
rjacobs CreditAttribution: rjacobs commentedSounds good, thanks. I started a follow-up issue here: #2422123: should entity view hooks be triggered unconditionally in node_view, term_view and user_view pages?.
Comment #43
kristofferwiklund CreditAttribution: kristofferwiklund commentedThis commit has introduced some bugs related to rendering display suite nodes in a panel for node/%node
Comment #44
kristofferwiklund CreditAttribution: kristofferwiklund commentedSeems to be the problem mention in #40.
Comment #45
rjacobs CreditAttribution: rjacobs commentedNote that there is some discussion in #2422123: should entity view hooks be triggered unconditionally in node_view, term_view and user_view pages? about reverting the changes from this issue, which could apply to ctools 7.x-1.8. The justification is that:
At this point comments on all this are probably best directed to #2422123: should entity view hooks be triggered unconditionally in node_view, term_view and user_view pages?.