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().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

brianV’s picture

I was able to work around this by implementing the following:

/**
 * Implements hook_gigya_comments_alter(). 
 */
function mymodule_gigya_comments_alter(&$params) {
  if (arg(0) == 'node' && is_numeric(arg(1))){
    $params['streamID'] = arg(1);
  }
}

sokrplare’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
961 bytes

Ran 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.

sokrplare’s picture

Title: Gigya Stream ID wrong on panels page containing view » Gigya Stream ID uses last node loaded on page NID
eyeless’s picture

Attached is I think a proper solution.

sokrplare’s picture

Interesting 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.

eyeless’s picture

Yes, 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.

sokrplare’s picture

I 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.

benjy’s picture

I think a panel pane would be a better way to go, something like this:

<?php

/**
 * @file
 * Gigya comment content type plugin.
 */

$plugin = array(
  'title' => t('Gigya Comments'),
  'single' => TRUE,
  'render callback' => 'cp_gigya_comment_pane_render',
  'description' => t('Provide previous and next navigation for videos.'),
  'required context' => new ctools_context_required(t('Video'), 'node'),
  'admin title' => 'Gigya Comments',
  'category' => array(t('Comment'), 0),
);


/**
 * Run-time rendering of the body of the block (content type).
 *
 * @param string $subtype
 *   The subtype.
 * @param array $conf
 *   Configuration as done at admin time.
 * @param array $args
 *   The render arguments.
 * @param object $context
 *   Context - in this case we don't have any
 *
 * @return object
 *   An object with at least title and content members.
 */
function cp_gigya_comment_pane_render($subtype, $conf, $args, $context) {
  $node = $context->data;

  $block = new stdClass();
  $block->title = "";
  $block->content = "<div id='comments'></div>";

  // Load the gigya stuff.
  module_load_include('inc', 'gigya', 'includes/gigya_comments');
  gigya_comments_add_ui($node->nid, $node->type);

  return $block;
}
?>
sokrplare’s picture

Sorry, 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.

benjy’s picture

@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.

luciodiri’s picture

Status: Needs review » Closed (fixed)

A 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.

Sam152’s picture

Priority: Normal » Critical
Status: Closed (fixed) » Needs work

This 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:

/**
 * Implements hook_node_view().
 */
function gigya_node_view($node, $view_mode, $langcode) {
  module_load_include('inc', 'gigya', 'includes/gigya_comments');
  gigya_add_comments_ui($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:

function gigya_add_comments_ui(&$node) {
  // Check if comments component is enabled.
  if (_gigya_is_component_enabled('comments') && empty($gigya_comments_ui_added)) {
    static $gigya_comments_ui_added = TRUE;

However a quick proof of concept shows that you must declare 'gigya_comments_ui_added' as static before checking if the variable is empty:

function foo() {
  if (empty($bar)) {
    echo 'I am printed on every function call.';
  }
  static $bar = TRUE;
}
foo();
foo();
foo();

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.

luciodiri’s picture

Status: Needs work » Needs review
FileSize
815 bytes

@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 :

/**
 * Implements hook_node_view().
 */
function gigya_node_view($node, $view_mode, $langcode) {
  if ($view_mode == 'full') {
    module_load_include('inc', 'gigya', 'includes/gigya_comments');
    gigya_add_comments_ui($node);
  }
}