If you are using PageManager to display nodes and your node has more comments than shown on one page, the pager at the bottom does not work.

It always shows comments from the first page, but the pager itself moves back and forward correctly in its own display.

Comments

slowflyer’s picture

Status: Active » Needs work
slowflyer’s picture

Status: Needs work » Active
swigle’s picture

+1 subscribing. Have exact the same issue.

aChris07’s picture

Subscribed. Same issue here, just as reported.
I've temporarily sidestepped the issue by implementing a custom pager for comments, but it's definitely not a fix.

marpic’s picture

Same issue here. aChris07 could you please share your workaround, it could be useful for me too.

Thanks

aChris07’s picture

@marpic Sorry, hadn't seen your reply.

I basically replicated the core logic, except I don't extend the integrated pager method in the query, but rather call all the comments of a thread and then page it myself. It might probably be not very efficient on very large threads, but it's been working fine for me so far, considering patching the issue might take more time than I have.

Any suggestion or update on the topic would be most welcome.

/**
 * Implements hook_preprocess_comment_wrapper().
 */
function module_preprocess_comment_wrapper(&$vars) {
  if (isset($vars['node']) && $vars['node']->type == 'forum') {
    // Replaces the forum node's paged comments with the full comment list
    // and uses it to implement a custom pager, given the pager generated by
    // the core isn't working:
    // https://www.drupal.org/node/2433923
    // @todo Fix or patch the ctools issue with the pager and remove this.
    
    // Load the list of thread comments.
    $thread_comments = module_load_node_comments($vars['node']);
    if (empty($thread_comments)) {
      return;
    }

    // Get the number of comments per page.
    $per_page = variable_get('comment_default_per_page_forum');
    // Initialize the pager.
    $current_page = pager_default_initialize($vars['node']->comment_count, $per_page);
    $vars['bottom_pager'] = theme('pager');
    $paged_comments = array();
    // Extract the list of comments from the array
    // (since it comes with a pager and sorted key-value).
    foreach($thread_comments as $comment_id => $comment) {
      if (is_numeric($comment_id)) {
        $paged_comments[$comment_id] = $comment;
        if (isset($vars['content']['comments'][$comment_id])) {
          unset($vars['content']['comments'][$comment_id]);
        }
      }
    }
    // Paginate the comments.
    $paged_comments = array_chunk($paged_comments, $per_page, TRUE);
    // Initialize the current comment page for node rendering.
    $final_comments = array();
    if (!empty($paged_comments[$current_page])) {
      comment_prepare_thread($paged_comments[$current_page]);
      $final_comments = comment_view_multiple($paged_comments[$current_page], $vars['node']);
    }
    // Set the current page of comments to be shown.
    $vars['content']['comments'] += $final_comments;
  }
}

/**
 * Loads all the comments of a node.
 */
function module_load_node_comments($node) {
  $mode = variable_get('comment_default_mode_forum', 0);
  $query = db_select('comment', 'c');
  $query->addField('c', 'cid');
  $query
    ->condition('c.nid', $node->nid)
    ->addTag('node_access')
    ->addTag('comment_filter')
    ->addMetaData('node', $node);
  if (!user_access('administer comments')) {
    $query->condition('c.status', COMMENT_PUBLISHED);
  }
  if ($mode === COMMENT_MODE_FLAT) {
    $query->orderBy('c.cid', 'ASC');
  }
  else {
    $query->addExpression('SUBSTRING(c.thread, 1, (LENGTH(c.thread) - 1))', 'torder');
    $query->orderBy('torder', 'ASC');
  }
  $result = $query->execute()->fetchCol();
  $comments = array();
  if (!empty($result)) {
    $comments = comment_load_multiple($result);
  }
  return $comments;
}
scottalan’s picture

I know why it's not working and have a fix but not sure it's the "right" one. It would require a patch the core comment module. I'm wondering if there is a way to do this without a patch...?

I'll explain my findings in brevity:

When comments are being loaded there is a call to comment_get_thread() and when comments are added to a page manager page that function is called again. This is basically where it all stems from.
http://cgit.drupalcode.org/drupal/tree/modules/comment/comment.module?h=...

Now if we go to includes/pager.inc the execute method calls $this->ensureElement()
http://cgit.drupalcode.org/drupal/tree/includes/pager.inc?h=7.x#n59

If you look at that method: http://cgit.drupalcode.org/drupal/tree/includes/pager.inc?h=7.x#n90
You will see that it increments $this->element with the static $maxElement

The first time this runs $this->element == 0, The next time it's ran $this->element == 1.

My guess is, it thinks this is a different pager. Therefore, it thinks it's still on the first page.

scottalan’s picture

StatusFileSize
new2.83 KB

This patch will fix the comment pager when adding comments via node_context/node_comments and node_context/node_comment_wrapper and you don't have to hack the comment wrapper :).

But...This also requires a patch to the core comment.module as well @see https://www.drupal.org/node/2472611

scottalan’s picture

Title: Comment Pager not working » Comment Pager not working when using node_comments or node_comment_wrapper plugins
Version: 7.x-1.6 » 7.x-1.x-dev
mpaler’s picture

Hi,

Was having the same exact issues with my installation. Ctools/Panels managed node pages with lots of comments, comment pager funky...even started my own core bug report. Anyhow, sadly, I applied the patch in #8 and the core patch and i was still seeing the issue. Not sure what else to report. Will help if I can.

Mike

paulihuhtiniemi’s picture

I have been troubleshooting this issue. In version 1.6-rc1 things still work, but 1.6 and 1.7 are broken.

comment_form() function is run twice and thus comment form has ID "comment-form--2" and page has two pagers ($pager_page_array). Then again, only one comment element is actually rendered on page, but behind the scenes comments seem to get processed twice.

paulihuhtiniemi’s picture

For me it looks like this commit introduced the problem: http://cgit.drupalcode.org/ctools/commit/?id=05d7202ef562b90fa1cf8c541ac... "Issue #1760384 by DamienMcKenna, meba: Update node_view.inc to execute the normal Drupal hooks"

paulihuhtiniemi’s picture

Patch from #2422123: should entity view hooks be triggered unconditionally in node_view, term_view and user_view pages? fixed this issue for me. Please test if it works for you too. If it does, we can close this issue as duplicate.

mpaler’s picture

Patch referenced in #14 fixed my issue. Thanks!

mpotter’s picture

I think the patch in #14 fixed it for the "comments" widget. But I'm still having trouble with the comment_wrapper widget not showing a pager at all. Might be just my dev system so curious if other people are seeing this.

rjacobs’s picture

Also, if anyone formally reviews any patches from #2422123: should entity view hooks be triggered unconditionally in node_view, term_view and user_view pages? as part of this issue, please be sure to post your findings in #2422123 as well. I think the solution pending there is solid but there has not been a lot of authoritative feedback on the technical specifics of the proposed solution/patch. Any help moving that issue forward is much appreciated.

func0der’s picture

Well I find the real problem in being the page manager calling everything twice.

Also I got a little plot twist for you: I have the same issue, but it is only happening if I am logged in and I almost all times have only two pages showing up, but both with the same comment content.
Does that change anything maybe? Or do you not even have that problem?

func0der’s picture

StatusFileSize
new1.32 KB

This would also fix the whole problem. It does not require a core fork or patch and the only down-side to it is, that the "page" parameter looks presumingly like this "1,1", "1,2" etc.

Take a look please. Not ideal, but at least core-fork-less.

func0der’s picture

StatusFileSize
new1.19 KB

Sorry, guys. Wrong paths.

Edit: Okay, wrong paths again. Please apply with `patch -p2`

khoomy’s picture

@func0der great, it works for me. But pager query string is encoded as ...?page=2%2C0%2C1, when I try to change manually in URL as ?page=2 then it does not work and shows only first page.

func0der’s picture

@khoomy
If you would url decode the url you would see that "%2C" would decode to a ",".
I mentioned in #19 that that is a side effect of this whole process. Pagers are not working like "page=2" anymore, but for example like "page=1,2" or in your case "page=2,0,1".
I do not know why you have only 3 numbers, I guess one of your pagers is broken.
The first integer identifies the pager index and the second one the page of that pager.

Your pagers may look a bit odd, but at least they work without a fork ;)

Andrew Edwards’s picture

StatusFileSize
new1.16 KB

Not sure why, but #20 didn't apply for me against the latest dev. Here's a new patch.

func0der’s picture

Yeah, sorry. I accidentally left "ctools" in the path.

@Andrew Edwards: Please provide interdiffs if you change existing patches. It is easier to follow the changes you have made and apply just those changes to the working copy.

socialnicheguru’s picture

Status: Active » Needs review

The last submitted patch, 20: ctools-fix_pager_issue-2433923-20.patch, failed testing.

plopesc’s picture

Status: Needs review » Reviewed & tested by the community

Patch #23 works for me.

Thanks!

func0der’s picture

Status: Reviewed & tested by the community » Needs work

@Plopesc

My patch is NOT!!! intended to be a solution for the problem.
It is just a WORKAROUND until someone finds a real solution to this problem.

I do not now think, that this should go in the the real module, but if that is what you want, so it be.
At least the pagers work somehow then.

func0der’s picture

Status: Needs work » Reviewed & tested by the community
japerry’s picture

Priority: Major » Normal
Status: Reviewed & tested by the community » Needs work

Marking needs work per #20 and #28. The patch doesn't seem right in a general sense. But if it fixes some people's local issues, thats great. It'll need more eyes and a revised patch before going in.

webpavilion’s picture

Assigned: Unassigned » webpavilion
StatusFileSize
new2.27 KB

WORKAROUND from #23 for node_comments and node_comment_wrapper

zkrebs’s picture

for what it's worth, i applied to the current ctools stable d7 release;

patch -p1 < ctools-fix_pager_issue-2433923-31.patch
patching file plugins/content_types/node_context/node_comments.inc
Hunk #1 succeeded at 87 (offset 3 lines).
Hunk #2 succeeded at 100 (offset 3 lines).
patching file plugins/content_types/node_context/node_comment_wrapper.inc
Hunk #1 succeeded at 43 (offset 4 lines).

It resolved the issue with my pager