We're experiencing an issue on a client site where we are getting incorrect stream ids on certain pages.
The pages in question are panel pages that sit in place of the node templates at node/. One pane contains the node content, another contains the comment form pane, with a div id set to match the one gigya targets in it's config.
A third pane contains a view that shows a list of related products using a view mode.
The streamID ends up being set to the node id of the last product in the related products view rather than the node id of the node we are viewing.
It appears that hook_form_comment_form_alter() may be getting invoked for the nodes contained in the related products view which each in turn overwrite the streamID due to that hook being implemented via gigya_form_comment_form_alter().
Comment | File | Size | Author |
---|---|---|---|
#13 | gigya-comment-streamID-1985636-13.patch | 815 bytes | luciodiri |
#4 | gigya-comment-streamID-1985636-4.patch | 504 bytes | eyeless |
#2 | gigya-comment-streamID-1985636-2.patch | 961 bytes | sokrplare |
Comments
Comment #1
brianV CreditAttribution: brianV commentedI was able to work around this by implementing the following:
Comment #2
sokrplare CreditAttribution: sokrplare commentedRan into this issue as well. The trouble is this code path gets triggered by hook_node_view() which can be called for multiple nodes on the same page. For our site, the Apache Solr "More Like This" block uses node rendering and so instead of the main content node on the page being used, the streamID was set to the NID for the very last node shown in the "More Like This" block.
I think a revamp might be more in order to trigger using something other than hook_node_view(), but until that happens, this fix should do the trick - it has for us.
Comment #3
sokrplare CreditAttribution: sokrplare commentedComment #4
eyeless CreditAttribution: eyeless commentedAttached is I think a proper solution.
Comment #5
sokrplare CreditAttribution: sokrplare commentedInteresting approach. I like it - so the assumption is that any other node on the page is going to be in a teaser view mode or something that has the comments disabled? It isn't guaranteed, but seems pretty likely in any normal use case.
Comment #6
eyeless CreditAttribution: eyeless commentedYes, this is not a complete solution, but it's because one more improvement is needed - to add support of multiple comments containers. Right now it is assumed to be one per page.
Comment #7
sokrplare CreditAttribution: sokrplare commentedI ran into an interesting side-effect once I patched this (using my patch in comment #2). Suddenly the "Log in or register to post comments" started popping up on the site where the teaser / other display modes were used for nodes. Because the original code was non-discriminating it ran them all through the Gigya comments code and unset those along the way - which was wrong but resulted in desired behavior in this case. Had to add a little one-off function in our utility module to hide those for some display modes.
While I was in there I tried the patch in comment 4, but it didn't ever get inside the if-statement for me. I didn't dig too far in, but it could be a conflict with the Commentsblock module or something else, or possibly just a bug.
Comment #8
benjy CreditAttribution: benjy commentedI think a panel pane would be a better way to go, something like this:
Comment #9
sokrplare CreditAttribution: sokrplare commentedSorry, not sure I follow, Bengy - was this the right issue for your comment? Not all pages are going to use panels so rendering as a pane would be handy on those that do, but leaves the rest stuck.
Comment #10
benjy CreditAttribution: benjy commented@covenantd sure, my post was specifically for people looking for a way to render the comments on a panel page. Adding it as a pane gives you the flexibility to re-order from the UI etc. There is no reason why the Gigya module couldn't provide a comment pane to be used with panels as well as their current approach.
Comment #11
luciodiri CreditAttribution: luciodiri commentedA more robust Panels support was introduced in V# 4.6
New functionality includes setting comments stream ID in Panels.
Thanks all for the constructive ideas.
Comment #12
Sam152 CreditAttribution: Sam152 commentedThis is still a large and active issue. Gigya's current implementation in 7.x-4.x the following code is used to attach the gigya comments to a node:
The problem is that hook_node_view is called when displaying any view mode (including teasers). The result is, the last node rendered on the page, regardless if comments are being rendered for that node and view mode is setting the streamID for the comments.
The correct approach would be to only add the JS setting inside the rendering specifically related to the gigya comments (ie, however the comments div is added to the page, should also setup the corresponding javascript variables) and not be done on a global level like hook_node_view().
Furthermore, you can see some attempt is made to only add the comment settings once:
However a quick proof of concept shows that you must declare 'gigya_comments_ui_added' as static before checking if the variable is empty:
The result of this bug is, any time a teaser is rendered after the "page node" comments, the streamID is incorrect. This explains the behaviour mentioned in the other comments and demonstrates a fundamental flaw in the way comments are attached and displayed on nodes.
Comment #13
luciodiri CreditAttribution: luciodiri commented@Sam152 A different approach for rendering the comments div would be tested for next release.
As an alternate temp patch, consider adding a condition to test $view_mode before calling gigya_add_comments_ui :