The call to pager_query on line 1007 and theme('pager' on line 1053 use an $element of 0. When this is called from a node which is displayed on a page with pagination (ie rendering comments on the front page), pager links for that page are lost because element 0 is already used.

CommentFileSizeAuthor
#2 comment_pager_conflict.patch1.45 KBcburschka

Comments

tara@neier.org’s picture

tara@neier.org commented
Assigned: Unassigned » tara@neier.org

Why couldn't this use an element of 2 instead. It's just as arbitrary, but it just happens to not break other pagination. I"m surprised this bug hasn't been opened earlier.
http://drupal.org/node/100529
http://drupal.org/node/71118

cburschka’s picture

cburschka
they
commented
Status: Active » Needs review
StatusFileSize
new comment_pager_conflict.patch1.45 KB

I'd rather use 1, unless there is an instance of 1 already being used elsewhere. Reason: If this issue crops up again in another paginating module, it would be good to have a logical progression and not to have to wonder why only 0 and 2 are being used so far.

That said, perhaps it would make sense to change the documentation to make a "reserved" block of pager element ids that must not be used by contribs?

The attached patch is rolled against DRUPAL-5 and changes the hard-coded value of 0 to a hard-coded value of 1. It does this only in the two places named (1007 and 1053).

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

What configuration of Drupal core only makes this possible? I am not aware of any way to make comments visible on the front page along with a post pager.

It would be best to keep this stable in Drupal 5 since this does change URLs and affects modules which use a pager in slot 1 on node pages.

joshmiller’s picture

joshmiller
he/him
Primary language English
Location Indianapolis, Indiana, USA
commented
Version: 5.1 » 7.x-dev
Assigned: tara@neier.org » Unassigned
Status: Needs work » Postponed (maintainer needs more info)

I just tried to apply this patch to Drupal 7 HEAD with no luck. (2 out of 2 hunks FAILED). After reading over the issue, it sounds like this "bug" needs a better description and use case. Updating the status.

josh

sun.core’s picture

sun.core commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Sorry, without further information this issue can only be marked as won't fix.

Feel free to re-open this issue if you want to provide further information. Thanks.